unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Volunteering to help on etc/TODO item: Improved xwidgets support
@ 2022-10-18 15:55 Andrew De Angelis
  2022-10-19  1:33 ` Po Lu
  0 siblings, 1 reply; 14+ messages in thread
From: Andrew De Angelis @ 2022-10-18 15:55 UTC (permalink / raw)
  To: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 2604 bytes --]

Hello everyone and thanks for all your work!

This is regarding TODO item: "Things to be done for specific packages or
features/NeXTstep port/Missing features/Improved xwidgets support"

I've started working on the NS code for xwidget-webkit, with the aim of
bringing it up-to-date with the changes to the X11 and GTK code (you can
check my as-yet-still-very-minor changes at this fork
<https://github.com/andyjda/emacs/tree/xwidget-patches>).
I'm sending the email to:
1) check if someone is already working on this
2) make sure I'm going about it the right way
3) inquire about the current X11/GTK implementation.

Regarding 1:
I haven't found many recent matches for 'xwidget' in the mailing list, but
if you're aware of someone already working on this effort, please let me
know

Regarding 2:
As noted in the Contributing node
<https://www.gnu.org/software/emacs/manual/html_node/emacs/Contributing.html>
of the manual, I'm making you aware of my planned improvements and I'd like
to know if you have any suggestions/advice. My current plan is to go
through the *xwidget.c* code, take note of any functions/subroutines that
are defined for GTK but not NS, and add an NS implementation in *xwidget.m*.
I will do my best to complete this so that the NS code will be fully
up-to-date. If there are any planned changes to *xwidget.c* or *xwidget.el *for
the upcoming 29.1 release, please let me know.
I don't know if I'll be able to have this ready for the 29.1 release, but I
can keep you up to speed on my progress.

Regarding 3:
I do not have a Linux machine available at the moment, which would be
valuable to get a better sense of the current GTK implementation (I'm
working on finding additional volunteers to help on this).
Is there a standard-procedure I can follow to ask questions here about the
GTK implementation? Is there a point person I should contact specifically?
I would like to keep the two different implementations as consistent as
possible, while also making sure that common bugs are addressed.
One question I have regarding this is on the `xwidget-webkit--loading-p'
variable: in my build, I see that this is set to true when creating a new
session, but it is then never updated to nil (even long after the web page
has fully loaded). Since this variable is not present in the C code, I'm
not sure if this is a limitation of the Lisp code (and therefore common
regardless of the underlying framework, GTK or NS), or if it's handled
correctly in other builds.

Thanks for reading this long email, feel free to address each of the three
different points individually.

Best,
Andrew

[-- Attachment #2: Type: text/html, Size: 3042 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Volunteering to help on etc/TODO item: Improved xwidgets support
  2022-10-18 15:55 Volunteering to help on etc/TODO item: Improved xwidgets support Andrew De Angelis
@ 2022-10-19  1:33 ` Po Lu
  2022-10-19  3:43   ` Andrew De Angelis
  0 siblings, 1 reply; 14+ messages in thread
From: Po Lu @ 2022-10-19  1:33 UTC (permalink / raw)
  To: Andrew De Angelis; +Cc: emacs-devel, Qiantan Hong

Andrew De Angelis <bobodeangelis@gmail.com> writes:

> Hello everyone and thanks for all your work!

Hello, thanks for working on Emacs.

> This is regarding TODO item: "Things to be done for specific packages
> or features/NeXTstep port/Missing features/Improved xwidgets support"
>
> I've started working on the NS code for xwidget-webkit, with the aim
> of bringing it up-to-date with the changes to the X11 and GTK code
> (you can check my as-yet-still-very-minor changes at this fork).  I'm
> sending the email to: 1) check if someone is already working on this
> 2) make sure I'm going about it the right way 3) inquire about the
> current X11/GTK implementation.
>
> Regarding 1: I haven't found many recent matches for 'xwidget' in the
> mailing list, but if you're aware of someone already working on this
> effort, please let me know

Sure.  It would be good if you coordinated your efforts with Qiantian
Hong (copied), who is also making changes in that area.

> Regarding 2: As noted in the Contributing node of the manual, I'm
> making you aware of my planned improvements and I'd like to know if
> you have any suggestions/advice. My current plan is to go through the
> xwidget.c code, take note of any functions/subroutines that are
> defined for GTK but not NS, and add an NS implementation in xwidget.m.

Thanks.  It would be nice if someone resolved the more crucial issues
first, though:

  - many procedures in nsxwidget.m crash when encountering killed
    xwidgets (see the doc string of `kill-xwidget' for more details.)
  - nsxwidget.m has apparently been written with Objective-C Automatic
    Reference Counting in mind, and thus leak memory, as Emacs cannot
    use ARC.

> I will do my best to complete this so that the NS code will be fully
> up-to-date. If there are any planned changes to xwidget.c or
> xwidget.el for the upcoming 29.1 release, please let me know.

I think it will probably be too late for Emacs 29.1, which will start
the pre-release process soon, at which point changes that don't only fix
regressions (and possibly the MS-DOS build) will not be allowed.  Unless
you can complete this work before November, that is.

> Regarding 3: I do not have a Linux machine available at the moment,
> which would be valuable to get a better sense of the current GTK
> implementation (I'm working on finding additional volunteers to help
> on this).  Is there a standard-procedure I can follow to ask questions
> here about the GTK implementation?

Just send an email to this list, with me copied in.  BTW, the canonical
implementation is not a "GTK implementation", but rather two
implementations combined in a single file through ifdefs: one for the
regular X11 build, and the other for a GTK build that does not use any X
Windows specific interfaces.

> Is there a point person I should contact specifically?

Me.

> I would like to keep the two different implementations as consistent
> as possible, while also making sure that common bugs are addressed.
> One question I have regarding this is on the
> `xwidget-webkit--loading-p' variable: in my build, I see that this is
> set to true when creating a new session, but it is then never updated
> to nil (even long after the web page has fully loaded). Since this
> variable is not present in the C code, I'm not sure if this is a
> limitation of the Lisp code (and therefore common regardless of the
> underlying framework, GTK or NS), or if it's handled correctly in
> other builds.

I think the timer stops by itself, the variable is not all that
important.



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Volunteering to help on etc/TODO item: Improved xwidgets support
  2022-10-19  1:33 ` Po Lu
@ 2022-10-19  3:43   ` Andrew De Angelis
  2022-10-19  4:45     ` Po Lu
  2022-10-19 18:54     ` Qiantan Hong
  0 siblings, 2 replies; 14+ messages in thread
From: Andrew De Angelis @ 2022-10-19  3:43 UTC (permalink / raw)
  To: Po Lu; +Cc: emacs-devel, Qiantan Hong

[-- Attachment #1: Type: text/plain, Size: 5511 bytes --]

Great, thanks for the informative reply!

It would be good if you coordinated your efforts with Qiantian
> Hong (copied), who is also making changes in that area.
>

Nice to meet you both. @Qiantian Hong let me know about your progress on
this, and if there's a specific area I should focus on, so we can combine
forces and avoid duplicate work.

Thanks.  It would be nice if someone resolved the more crucial issues
> first, though
>

I wasn't aware of these, but I will prioritize them.

Unless you can complete this work before November


Yeah, that's a tight deadline. I am going to try to fix the biggest issues
by then, but we'll probably have to wait until the next release to have
this included

I think the timer stops by itself, the variable is not all that
> important.
>

This is probably another NS-specific issue, but the timer doesn't always
stop in my build. The issue starts in `xwidget-webkit-callback'; it looks
like the  "load-finished" event sometimes (most of the times) doesn't
happen or is not received. I will add this to the list of things to fix.

One other possible Lisp bug: currently, `xwidget-webkit-current-url' always
returns "URL: nil" for me. This shouldn't depend on other xwidget code: the
issue is caused by the fact that `kill-new' doesn't return the string it
just added to the kill-ring. (If it does work in your builds let me know,
as I'd have to investigate what's causing such different behavior.)
This is my fix for this:

(defun xwidget-webkit-current-url ()
  "Display the current xwidget webkit URL and place it on the `kill-ring'."
  (interactive nil xwidget-webkit-mode)
  (let ((url (or (xwidget-webkit-uri (xwidget-webkit-current-session)) "")))
    (kill-new url)
    (message "URL: %s" url)))




On Tue, Oct 18, 2022 at 9:33 PM Po Lu <luangruo@yahoo.com> wrote:

> Andrew De Angelis <bobodeangelis@gmail.com> writes:
>
> > Hello everyone and thanks for all your work!
>
> Hello, thanks for working on Emacs.
>
> > This is regarding TODO item: "Things to be done for specific packages
> > or features/NeXTstep port/Missing features/Improved xwidgets support"
> >
> > I've started working on the NS code for xwidget-webkit, with the aim
> > of bringing it up-to-date with the changes to the X11 and GTK code
> > (you can check my as-yet-still-very-minor changes at this fork).  I'm
> > sending the email to: 1) check if someone is already working on this
> > 2) make sure I'm going about it the right way 3) inquire about the
> > current X11/GTK implementation.
> >
> > Regarding 1: I haven't found many recent matches for 'xwidget' in the
> > mailing list, but if you're aware of someone already working on this
> > effort, please let me know
>
> Sure.  It would be good if you coordinated your efforts with Qiantian
> Hong (copied), who is also making changes in that area.
>
> > Regarding 2: As noted in the Contributing node of the manual, I'm
> > making you aware of my planned improvements and I'd like to know if
> > you have any suggestions/advice. My current plan is to go through the
> > xwidget.c code, take note of any functions/subroutines that are
> > defined for GTK but not NS, and add an NS implementation in xwidget.m.
>
> Thanks.  It would be nice if someone resolved the more crucial issues
> first, though:
>
>   - many procedures in nsxwidget.m crash when encountering killed
>     xwidgets (see the doc string of `kill-xwidget' for more details.)
>   - nsxwidget.m has apparently been written with Objective-C Automatic
>     Reference Counting in mind, and thus leak memory, as Emacs cannot
>     use ARC.
>
> > I will do my best to complete this so that the NS code will be fully
> > up-to-date. If there are any planned changes to xwidget.c or
> > xwidget.el for the upcoming 29.1 release, please let me know.
>
> I think it will probably be too late for Emacs 29.1, which will start
> the pre-release process soon, at which point changes that don't only fix
> regressions (and possibly the MS-DOS build) will not be allowed.  Unless
> you can complete this work before November, that is.
>
> > Regarding 3: I do not have a Linux machine available at the moment,
> > which would be valuable to get a better sense of the current GTK
> > implementation (I'm working on finding additional volunteers to help
> > on this).  Is there a standard-procedure I can follow to ask questions
> > here about the GTK implementation?
>
> Just send an email to this list, with me copied in.  BTW, the canonical
> implementation is not a "GTK implementation", but rather two
> implementations combined in a single file through ifdefs: one for the
> regular X11 build, and the other for a GTK build that does not use any X
> Windows specific interfaces.
>
> > Is there a point person I should contact specifically?
>
> Me.
>
> > I would like to keep the two different implementations as consistent
> > as possible, while also making sure that common bugs are addressed.
> > One question I have regarding this is on the
> > `xwidget-webkit--loading-p' variable: in my build, I see that this is
> > set to true when creating a new session, but it is then never updated
> > to nil (even long after the web page has fully loaded). Since this
> > variable is not present in the C code, I'm not sure if this is a
> > limitation of the Lisp code (and therefore common regardless of the
> > underlying framework, GTK or NS), or if it's handled correctly in
> > other builds.
>
> I think the timer stops by itself, the variable is not all that
> important.
>

[-- Attachment #2: Type: text/html, Size: 7287 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Volunteering to help on etc/TODO item: Improved xwidgets support
  2022-10-19  3:43   ` Andrew De Angelis
@ 2022-10-19  4:45     ` Po Lu
  2022-10-19  4:55       ` Andrew De Angelis
  2022-10-19 18:54     ` Qiantan Hong
  1 sibling, 1 reply; 14+ messages in thread
From: Po Lu @ 2022-10-19  4:45 UTC (permalink / raw)
  To: Andrew De Angelis; +Cc: emacs-devel, Qiantan Hong

Andrew De Angelis <bobodeangelis@gmail.com> writes:

> Yeah, that's a tight deadline. I am going to try to fix the biggest
> issues by then, but we'll probably have to wait until the next release
> to have this included

Thanks.

>  I think the timer stops by itself, the variable is not all that
>  important.
>
> This is probably another NS-specific issue, but the timer doesn't
> always stop in my build. The issue starts in
> `xwidget-webkit-callback'; it looks like the "load-finished" event
> sometimes (most of the times) doesn't happen or is not received. I
> will add this to the list of things to fix.

Yeah, that sounds like an NS specific bug.

> One other possible Lisp bug: currently, `xwidget-webkit-current-url'
> always returns "URL: nil" for me. This shouldn't depend on other
> xwidget code: the issue is caused by the fact that `kill-new' doesn't
> return the string it just added to the kill-ring. (If it does work in
> your builds let me know, as I'd have to investigate what's causing
> such different behavior.)  This is my fix for this:

It should never print "nil"; it's a command that is not supposed to be
called by Lisp or return anything.



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Volunteering to help on etc/TODO item: Improved xwidgets support
  2022-10-19  4:45     ` Po Lu
@ 2022-10-19  4:55       ` Andrew De Angelis
  2022-10-19  5:11         ` Po Lu
  0 siblings, 1 reply; 14+ messages in thread
From: Andrew De Angelis @ 2022-10-19  4:55 UTC (permalink / raw)
  To: Po Lu; +Cc: emacs-devel, Qiantan Hong

[-- Attachment #1: Type: text/plain, Size: 1774 bytes --]

>
> It should never print "nil"; it's a command that is not supposed to be
> called by Lisp or return anything.
>
Sorry, I misspoke: the issue is that it *prints* nil: if I have a widget
open, and press 'w' or call `xwidget-webkit-current-url', the message I see
in the minibuffer is always "URL: nil".
This happens even though (xwidget-webkit-uri (xwidget-webkit-current-
session))  does return the current url. Therefore, I think the problem is
in how the function uses `kill-new'.

On Wed, Oct 19, 2022 at 12:46 AM Po Lu <luangruo@yahoo.com> wrote:

> Andrew De Angelis <bobodeangelis@gmail.com> writes:
>
> > Yeah, that's a tight deadline. I am going to try to fix the biggest
> > issues by then, but we'll probably have to wait until the next release
> > to have this included
>
> Thanks.
>
> >  I think the timer stops by itself, the variable is not all that
> >  important.
> >
> > This is probably another NS-specific issue, but the timer doesn't
> > always stop in my build. The issue starts in
> > `xwidget-webkit-callback'; it looks like the "load-finished" event
> > sometimes (most of the times) doesn't happen or is not received. I
> > will add this to the list of things to fix.
>
> Yeah, that sounds like an NS specific bug.
>
> > One other possible Lisp bug: currently, `xwidget-webkit-current-url'
> > always returns "URL: nil" for me. This shouldn't depend on other
> > xwidget code: the issue is caused by the fact that `kill-new' doesn't
> > return the string it just added to the kill-ring. (If it does work in
> > your builds let me know, as I'd have to investigate what's causing
> > such different behavior.)  This is my fix for this:
>
> It should never print "nil"; it's a command that is not supposed to be
> called by Lisp or return anything.
>

[-- Attachment #2: Type: text/html, Size: 2652 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Volunteering to help on etc/TODO item: Improved xwidgets support
  2022-10-19  4:55       ` Andrew De Angelis
@ 2022-10-19  5:11         ` Po Lu
  0 siblings, 0 replies; 14+ messages in thread
From: Po Lu @ 2022-10-19  5:11 UTC (permalink / raw)
  To: Andrew De Angelis; +Cc: emacs-devel, Qiantan Hong

Andrew De Angelis <bobodeangelis@gmail.com> writes:

>  It should never print "nil"; it's a command that is not supposed to be
>  called by Lisp or return anything.
>
> Sorry, I misspoke: the issue is that it prints nil: if I have a widget open, and press 'w' or call `xwidget-webkit-current-url', the message I see in
> the minibuffer is always "URL: nil". 
> This happens even though (xwidget-webkit-uri (xwidget-webkit-current-session))  does return the current url. Therefore, I think the problem is
> in how the function uses `kill-new'.

Nevermind, you're right.  I was looking at a different copy of the code.



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Volunteering to help on etc/TODO item: Improved xwidgets support
  2022-10-19  3:43   ` Andrew De Angelis
  2022-10-19  4:45     ` Po Lu
@ 2022-10-19 18:54     ` Qiantan Hong
  2022-10-20  2:23       ` Andrew De Angelis
  1 sibling, 1 reply; 14+ messages in thread
From: Qiantan Hong @ 2022-10-19 18:54 UTC (permalink / raw)
  To: Andrew De Angelis; +Cc: Po Lu, emacs-devel@gnu.org

Hi Andrew,

Nice to meet you! Yes I was working on adding user content API to xwidgets.

You can find a version of my patch at https://lists.gnu.org/archive/html/emacs-devel/2022-10/msg01212.html
The patch has many issues and I’m currently working on fixing it (I can only work
on weekends unfortunately), but that’s mostly on the common/gtk part. The NS part
is mostly finalized. However, it does share the exact same issue as all other NS code
-- it's written in ARC style so it leaks memory. I'm not familiar with NS without ARC, so
it will be nice if you can help fixing it!

Feel free to make patch to any existing NS code -- I'm not touching them that much.

Best,
Qiantan



> On Oct 18, 2022, at 8:43 PM, Andrew De Angelis <bobodeangelis@gmail.com> wrote:
> 
> Great, thanks for the informative reply!
> 
> It would be good if you coordinated your efforts with Qiantian
> Hong (copied), who is also making changes in that area.
> 
> Nice to meet you both. @Qiantian Hong let me know about your progress on this, and if there's a specific area I should focus on, so we can combine forces and avoid duplicate work.
> 
> Thanks.  It would be nice if someone resolved the more crucial issues
> first, though
> 
> I wasn't aware of these, but I will prioritize them.
> 
> Unless you can complete this work before November 
> 
> Yeah, that's a tight deadline. I am going to try to fix the biggest issues by then, but we'll probably have to wait until the next release to have this included
> 
> I think the timer stops by itself, the variable is not all that
> important.
> 
> This is probably another NS-specific issue, but the timer doesn't always stop in my build. The issue starts in `xwidget-webkit-callback'; it looks like the  "load-finished" event sometimes (most of the times) doesn't happen or is not received. I will add this to the list of things to fix.
> 
> One other possible Lisp bug: currently, `xwidget-webkit-current-url' always returns "URL: nil" for me. This shouldn't depend on other xwidget code: the issue is caused by the fact that `kill-new' doesn't return the string it just added to the kill-ring. (If it does work in your builds let me know, as I'd have to investigate what's causing such different behavior.)
> This is my fix for this:
> 
> (defun xwidget-webkit-current-url ()
>   "Display the current xwidget webkit URL and place it on the `kill-ring'."
>   (interactive nil xwidget-webkit-mode)
>   (let ((url (or (xwidget-webkit-uri (xwidget-webkit-current-session)) "")))
>     (kill-new url)
>     (message "URL: %s" url)))
> 
>  
>  
> 
> On Tue, Oct 18, 2022 at 9:33 PM Po Lu <luangruo@yahoo.com> wrote:
> Andrew De Angelis <bobodeangelis@gmail.com> writes:
> 
> > Hello everyone and thanks for all your work!
> 
> Hello, thanks for working on Emacs.
> 
> > This is regarding TODO item: "Things to be done for specific packages
> > or features/NeXTstep port/Missing features/Improved xwidgets support"
> >
> > I've started working on the NS code for xwidget-webkit, with the aim
> > of bringing it up-to-date with the changes to the X11 and GTK code
> > (you can check my as-yet-still-very-minor changes at this fork).  I'm
> > sending the email to: 1) check if someone is already working on this
> > 2) make sure I'm going about it the right way 3) inquire about the
> > current X11/GTK implementation.
> >
> > Regarding 1: I haven't found many recent matches for 'xwidget' in the
> > mailing list, but if you're aware of someone already working on this
> > effort, please let me know
> 
> Sure.  It would be good if you coordinated your efforts with Qiantian
> Hong (copied), who is also making changes in that area.
> 
> > Regarding 2: As noted in the Contributing node of the manual, I'm
> > making you aware of my planned improvements and I'd like to know if
> > you have any suggestions/advice. My current plan is to go through the
> > xwidget.c code, take note of any functions/subroutines that are
> > defined for GTK but not NS, and add an NS implementation in xwidget.m.
> 
> Thanks.  It would be nice if someone resolved the more crucial issues
> first, though:
> 
>   - many procedures in nsxwidget.m crash when encountering killed
>     xwidgets (see the doc string of `kill-xwidget' for more details.)
>   - nsxwidget.m has apparently been written with Objective-C Automatic
>     Reference Counting in mind, and thus leak memory, as Emacs cannot
>     use ARC.
> 
> > I will do my best to complete this so that the NS code will be fully
> > up-to-date. If there are any planned changes to xwidget.c or
> > xwidget.el for the upcoming 29.1 release, please let me know.
> 
> I think it will probably be too late for Emacs 29.1, which will start
> the pre-release process soon, at which point changes that don't only fix
> regressions (and possibly the MS-DOS build) will not be allowed.  Unless
> you can complete this work before November, that is.
> 
> > Regarding 3: I do not have a Linux machine available at the moment,
> > which would be valuable to get a better sense of the current GTK
> > implementation (I'm working on finding additional volunteers to help
> > on this).  Is there a standard-procedure I can follow to ask questions
> > here about the GTK implementation?
> 
> Just send an email to this list, with me copied in.  BTW, the canonical
> implementation is not a "GTK implementation", but rather two
> implementations combined in a single file through ifdefs: one for the
> regular X11 build, and the other for a GTK build that does not use any X
> Windows specific interfaces.
> 
> > Is there a point person I should contact specifically?
> 
> Me.
> 
> > I would like to keep the two different implementations as consistent
> > as possible, while also making sure that common bugs are addressed.
> > One question I have regarding this is on the
> > `xwidget-webkit--loading-p' variable: in my build, I see that this is
> > set to true when creating a new session, but it is then never updated
> > to nil (even long after the web page has fully loaded). Since this
> > variable is not present in the C code, I'm not sure if this is a
> > limitation of the Lisp code (and therefore common regardless of the
> > underlying framework, GTK or NS), or if it's handled correctly in
> > other builds.
> 
> I think the timer stops by itself, the variable is not all that
> important.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Volunteering to help on etc/TODO item: Improved xwidgets support
  2022-10-19 18:54     ` Qiantan Hong
@ 2022-10-20  2:23       ` Andrew De Angelis
  2022-10-20  2:46         ` Po Lu
  2022-10-20 11:13         ` Daniel Martín
  0 siblings, 2 replies; 14+ messages in thread
From: Andrew De Angelis @ 2022-10-20  2:23 UTC (permalink / raw)
  To: Qiantan Hong; +Cc: Po Lu, emacs-devel@gnu.org

[-- Attachment #1: Type: text/plain, Size: 7493 bytes --]

Great, thanks Qiantian for the info.
I'm looking into ways to adapt the Objective C code to use manual reference
counting rather than ARC. Is this a problem that other Objective-C Emacs
files have? Maybe I can also come up with some general rules/approaches to
help people fix the problem in other files.
One thing that would be very helpful is if you (or anyone else) is aware of
somebody making this fix (ARC -> MRC in Objective C) before: most of the
tutorials/documentation I'm finding on the topic is on how to go in the
opposite direction, which is helpful information but I'm stuck "reverse
engineering" the whole approach.

On Wed, Oct 19, 2022 at 2:54 PM Qiantan Hong <qthong@stanford.edu> wrote:

> Hi Andrew,
>
> Nice to meet you! Yes I was working on adding user content API to xwidgets.
>
> You can find a version of my patch at
> https://lists.gnu.org/archive/html/emacs-devel/2022-10/msg01212.html
> The patch has many issues and I’m currently working on fixing it (I can
> only work
> on weekends unfortunately), but that’s mostly on the common/gtk part. The
> NS part
> is mostly finalized. However, it does share the exact same issue as all
> other NS code
> -- it's written in ARC style so it leaks memory. I'm not familiar with NS
> without ARC, so
> it will be nice if you can help fixing it!
>
> Feel free to make patch to any existing NS code -- I'm not touching them
> that much.
>
> Best,
> Qiantan
>
>
>
> > On Oct 18, 2022, at 8:43 PM, Andrew De Angelis <bobodeangelis@gmail.com>
> wrote:
> >
> > Great, thanks for the informative reply!
> >
> > It would be good if you coordinated your efforts with Qiantian
> > Hong (copied), who is also making changes in that area.
> >
> > Nice to meet you both. @Qiantian Hong let me know about your progress on
> this, and if there's a specific area I should focus on, so we can combine
> forces and avoid duplicate work.
> >
> > Thanks.  It would be nice if someone resolved the more crucial issues
> > first, though
> >
> > I wasn't aware of these, but I will prioritize them.
> >
> > Unless you can complete this work before November
> >
> > Yeah, that's a tight deadline. I am going to try to fix the biggest
> issues by then, but we'll probably have to wait until the next release to
> have this included
> >
> > I think the timer stops by itself, the variable is not all that
> > important.
> >
> > This is probably another NS-specific issue, but the timer doesn't always
> stop in my build. The issue starts in `xwidget-webkit-callback'; it looks
> like the  "load-finished" event sometimes (most of the times) doesn't
> happen or is not received. I will add this to the list of things to fix.
> >
> > One other possible Lisp bug: currently, `xwidget-webkit-current-url'
> always returns "URL: nil" for me. This shouldn't depend on other xwidget
> code: the issue is caused by the fact that `kill-new' doesn't return the
> string it just added to the kill-ring. (If it does work in your builds let
> me know, as I'd have to investigate what's causing such different behavior.)
> > This is my fix for this:
> >
> > (defun xwidget-webkit-current-url ()
> >   "Display the current xwidget webkit URL and place it on the
> `kill-ring'."
> >   (interactive nil xwidget-webkit-mode)
> >   (let ((url (or (xwidget-webkit-uri (xwidget-webkit-current-session))
> "")))
> >     (kill-new url)
> >     (message "URL: %s" url)))
> >
> >
> >
> >
> > On Tue, Oct 18, 2022 at 9:33 PM Po Lu <luangruo@yahoo.com> wrote:
> > Andrew De Angelis <bobodeangelis@gmail.com> writes:
> >
> > > Hello everyone and thanks for all your work!
> >
> > Hello, thanks for working on Emacs.
> >
> > > This is regarding TODO item: "Things to be done for specific packages
> > > or features/NeXTstep port/Missing features/Improved xwidgets support"
> > >
> > > I've started working on the NS code for xwidget-webkit, with the aim
> > > of bringing it up-to-date with the changes to the X11 and GTK code
> > > (you can check my as-yet-still-very-minor changes at this fork).  I'm
> > > sending the email to: 1) check if someone is already working on this
> > > 2) make sure I'm going about it the right way 3) inquire about the
> > > current X11/GTK implementation.
> > >
> > > Regarding 1: I haven't found many recent matches for 'xwidget' in the
> > > mailing list, but if you're aware of someone already working on this
> > > effort, please let me know
> >
> > Sure.  It would be good if you coordinated your efforts with Qiantian
> > Hong (copied), who is also making changes in that area.
> >
> > > Regarding 2: As noted in the Contributing node of the manual, I'm
> > > making you aware of my planned improvements and I'd like to know if
> > > you have any suggestions/advice. My current plan is to go through the
> > > xwidget.c code, take note of any functions/subroutines that are
> > > defined for GTK but not NS, and add an NS implementation in xwidget.m.
> >
> > Thanks.  It would be nice if someone resolved the more crucial issues
> > first, though:
> >
> >   - many procedures in nsxwidget.m crash when encountering killed
> >     xwidgets (see the doc string of `kill-xwidget' for more details.)
> >   - nsxwidget.m has apparently been written with Objective-C Automatic
> >     Reference Counting in mind, and thus leak memory, as Emacs cannot
> >     use ARC.
> >
> > > I will do my best to complete this so that the NS code will be fully
> > > up-to-date. If there are any planned changes to xwidget.c or
> > > xwidget.el for the upcoming 29.1 release, please let me know.
> >
> > I think it will probably be too late for Emacs 29.1, which will start
> > the pre-release process soon, at which point changes that don't only fix
> > regressions (and possibly the MS-DOS build) will not be allowed.  Unless
> > you can complete this work before November, that is.
> >
> > > Regarding 3: I do not have a Linux machine available at the moment,
> > > which would be valuable to get a better sense of the current GTK
> > > implementation (I'm working on finding additional volunteers to help
> > > on this).  Is there a standard-procedure I can follow to ask questions
> > > here about the GTK implementation?
> >
> > Just send an email to this list, with me copied in.  BTW, the canonical
> > implementation is not a "GTK implementation", but rather two
> > implementations combined in a single file through ifdefs: one for the
> > regular X11 build, and the other for a GTK build that does not use any X
> > Windows specific interfaces.
> >
> > > Is there a point person I should contact specifically?
> >
> > Me.
> >
> > > I would like to keep the two different implementations as consistent
> > > as possible, while also making sure that common bugs are addressed.
> > > One question I have regarding this is on the
> > > `xwidget-webkit--loading-p' variable: in my build, I see that this is
> > > set to true when creating a new session, but it is then never updated
> > > to nil (even long after the web page has fully loaded). Since this
> > > variable is not present in the C code, I'm not sure if this is a
> > > limitation of the Lisp code (and therefore common regardless of the
> > > underlying framework, GTK or NS), or if it's handled correctly in
> > > other builds.
> >
> > I think the timer stops by itself, the variable is not all that
> > important.
>
>

[-- Attachment #2: Type: text/html, Size: 9044 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Volunteering to help on etc/TODO item: Improved xwidgets support
  2022-10-20  2:23       ` Andrew De Angelis
@ 2022-10-20  2:46         ` Po Lu
  2022-10-20 11:13         ` Daniel Martín
  1 sibling, 0 replies; 14+ messages in thread
From: Po Lu @ 2022-10-20  2:46 UTC (permalink / raw)
  To: Andrew De Angelis; +Cc: Qiantan Hong, emacs-devel@gnu.org

Andrew De Angelis <bobodeangelis@gmail.com> writes:

> Great, thanks Qiantian for the info.  I'm looking into ways to adapt
> the Objective C code to use manual reference counting rather than
> ARC. Is this a problem that other Objective-C Emacs files have?

No, the other Objective-C files do not have this problem.

> Maybe I can also come up with some general rules/approaches to help
> people fix the problem in other files.  One thing that would be very
> helpful is if you (or anyone else) is aware of somebody making this
> fix (ARC -> MRC in Objective C) before: most of the
> tutorials/documentation I'm finding on the topic is on how to go in
> the opposite direction, which is helpful information but I'm stuck
> "reverse engineering" the whole approach.

Not having much experience with Objective-C, I can't say.  But I think
code like this:

      [scriptor addUserScript:[[WKUserScript alloc]
                                initWithSource:xwScript
                                 injectionTime:
                                  WKUserScriptInjectionTimeAtDocumentStart
                                forMainFrameOnly:NO]];

has to be modified to keep track of the allocated WKUserScript
objects, and to call release on them when necessary.



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Volunteering to help on etc/TODO item: Improved xwidgets support
  2022-10-20  2:23       ` Andrew De Angelis
  2022-10-20  2:46         ` Po Lu
@ 2022-10-20 11:13         ` Daniel Martín
  2022-10-21 23:29           ` Andrew De Angelis
  1 sibling, 1 reply; 14+ messages in thread
From: Daniel Martín @ 2022-10-20 11:13 UTC (permalink / raw)
  To: Andrew De Angelis; +Cc: Qiantan Hong, Po Lu, emacs-devel@gnu.org

Andrew De Angelis <bobodeangelis@gmail.com> writes:

> Great, thanks Qiantian for the info.
> I'm looking into ways to adapt the Objective C code to use manual reference
> counting rather than ARC. Is this a problem that other Objective-C Emacs
> files have? Maybe I can also come up with some general rules/approaches to
> help people fix the problem in other files.
> One thing that would be very helpful is if you (or anyone else) is aware of
> somebody making this fix (ARC -> MRC in Objective C) before: most of the
> tutorials/documentation I'm finding on the topic is on how to go in the
> opposite direction, which is helpful information but I'm stuck "reverse
> engineering" the whole approach.
>

I'd also suggest instrumenting Emacs with Instruments.app and the
"Leaks" profile.  The generated profile will indicate if there are
retain/release/autorelease problems that led to leaked objects under
the MRC model.

In recent versions of macOS you might have problems running Emacs under
Instruments.app.  One way to resolve this problem is by embedding a
custom entitlement that sets "com.apple.security.get-task-allow" and
then signing the binary again.  That is, run this command from a shell:

$ codesign -s - -v -f --entitlements Entitlements.plist emacs

In the command above, "emacs" is the built Emacs binary and
Entitlements.plist should contain this text:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "https://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
  <dict>
    <key>com.apple.security.get-task-allow
    </key>
    <true/>
  </dict>
</plist>

I suspect this tweak could perfectly be part of the Emacs build system
for NS, but I haven't investigated further.



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Volunteering to help on etc/TODO item: Improved xwidgets support
  2022-10-20 11:13         ` Daniel Martín
@ 2022-10-21 23:29           ` Andrew De Angelis
  2022-10-22  0:07             ` Po Lu
  0 siblings, 1 reply; 14+ messages in thread
From: Andrew De Angelis @ 2022-10-21 23:29 UTC (permalink / raw)
  To: Daniel Martín; +Cc: Qiantan Hong, Po Lu, emacs-devel@gnu.org

[-- Attachment #1: Type: text/plain, Size: 2271 bytes --]

Thanks Daniel! That's very helpful. I was looking at documentation online
regarding profiling Emacs, but most of it is GNU/Linux focused so they'd
all mention gprof, which to my understanding doesn't work on Mac.
I'll work on using the Instruments.app, that should be very helpful in
finding the leaks.

On Thu, Oct 20, 2022 at 7:13 AM Daniel Martín <mardani29@yahoo.es> wrote:

> Andrew De Angelis <bobodeangelis@gmail.com> writes:
>
> > Great, thanks Qiantian for the info.
> > I'm looking into ways to adapt the Objective C code to use manual
> reference
> > counting rather than ARC. Is this a problem that other Objective-C Emacs
> > files have? Maybe I can also come up with some general rules/approaches
> to
> > help people fix the problem in other files.
> > One thing that would be very helpful is if you (or anyone else) is aware
> of
> > somebody making this fix (ARC -> MRC in Objective C) before: most of the
> > tutorials/documentation I'm finding on the topic is on how to go in the
> > opposite direction, which is helpful information but I'm stuck "reverse
> > engineering" the whole approach.
> >
>
> I'd also suggest instrumenting Emacs with Instruments.app and the
> "Leaks" profile.  The generated profile will indicate if there are
> retain/release/autorelease problems that led to leaked objects under
> the MRC model.
>
> In recent versions of macOS you might have problems running Emacs under
> Instruments.app.  One way to resolve this problem is by embedding a
> custom entitlement that sets "com.apple.security.get-task-allow" and
> then signing the binary again.  That is, run this command from a shell:
>
> $ codesign -s - -v -f --entitlements Entitlements.plist emacs
>
> In the command above, "emacs" is the built Emacs binary and
> Entitlements.plist should contain this text:
>
> <?xml version="1.0" encoding="UTF-8"?>
> <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "
> https://www.apple.com/DTDs/PropertyList-1.0.dtd">
> <plist version="1.0">
>   <dict>
>     <key>com.apple.security.get-task-allow
>     </key>
>     <true/>
>   </dict>
> </plist>
>
> I suspect this tweak could perfectly be part of the Emacs build system
> for NS, but I haven't investigated further.
>

[-- Attachment #2: Type: text/html, Size: 3015 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Volunteering to help on etc/TODO item: Improved xwidgets support
  2022-10-21 23:29           ` Andrew De Angelis
@ 2022-10-22  0:07             ` Po Lu
  2022-10-22  2:13               ` Andrew De Angelis
  2022-10-22  4:49               ` Matt Armstrong
  0 siblings, 2 replies; 14+ messages in thread
From: Po Lu @ 2022-10-22  0:07 UTC (permalink / raw)
  To: Andrew De Angelis; +Cc: Daniel Martín, Qiantan Hong, emacs-devel@gnu.org

Andrew De Angelis <bobodeangelis@gmail.com> writes:

> Thanks Daniel! That's very helpful. I was looking at documentation
> online regarding profiling Emacs, but most of it is GNU/Linux focused
> so they'd all mention gprof, which to my understanding doesn't work on
> Mac.  I'll work on using the Instruments.app, that should be very
> helpful in finding the leaks.

Doesn't Valgrind work on Mac OS as well?



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Volunteering to help on etc/TODO item: Improved xwidgets support
  2022-10-22  0:07             ` Po Lu
@ 2022-10-22  2:13               ` Andrew De Angelis
  2022-10-22  4:49               ` Matt Armstrong
  1 sibling, 0 replies; 14+ messages in thread
From: Andrew De Angelis @ 2022-10-22  2:13 UTC (permalink / raw)
  To: Po Lu; +Cc: Daniel Martín, Qiantan Hong, emacs-devel@gnu.org

[-- Attachment #1: Type: text/plain, Size: 1219 bytes --]

Haven't worked with Valgrind in a while, and never on MacOS, so not sure. A
quick glance at online documentation here
<https://github.com/LouisBrunner/valgrind-macos> and here
<https://valgrind.org/> shows the latest mentioned versions to be a bit old
(10.12 and 10.15, I'm running 11.6.7 on my machine). But if you recommend
it I'll take a better look.

I was referring to this portion of the manual: link
<https://www.gnu.org/software/emacs/manual/html_node/elisp/Profiling.html>. It
seems Emacs is pretty well integrated with gprof. But on Mac I couldn't
configure it with the --enable-profiling option; I get this error: *clang:
error: the clang compiler does not support -pg option on versions of OS X
10.9 and later.*

On Fri, Oct 21, 2022 at 8:07 PM Po Lu <luangruo@yahoo.com> wrote:

> Andrew De Angelis <bobodeangelis@gmail.com> writes:
>
> > Thanks Daniel! That's very helpful. I was looking at documentation
> > online regarding profiling Emacs, but most of it is GNU/Linux focused
> > so they'd all mention gprof, which to my understanding doesn't work on
> > Mac.  I'll work on using the Instruments.app, that should be very
> > helpful in finding the leaks.
>
> Doesn't Valgrind work on Mac OS as well?
>

[-- Attachment #2: Type: text/html, Size: 1751 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Volunteering to help on etc/TODO item: Improved xwidgets support
  2022-10-22  0:07             ` Po Lu
  2022-10-22  2:13               ` Andrew De Angelis
@ 2022-10-22  4:49               ` Matt Armstrong
  1 sibling, 0 replies; 14+ messages in thread
From: Matt Armstrong @ 2022-10-22  4:49 UTC (permalink / raw)
  To: Po Lu, Andrew De Angelis
  Cc: Daniel Martín, Qiantan Hong, emacs-devel@gnu.org

Po Lu <luangruo@yahoo.com> writes:

> Andrew De Angelis <bobodeangelis@gmail.com> writes:
>
>> Thanks Daniel! That's very helpful. I was looking at documentation
>> online regarding profiling Emacs, but most of it is GNU/Linux focused
>> so they'd all mention gprof, which to my understanding doesn't work on
>> Mac.  I'll work on using the Instruments.app, that should be very
>> helpful in finding the leaks.
>
> Doesn't Valgrind work on Mac OS as well?

I think Address Sanitizer should work on Mac's clang as well.



^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-10-22  4:49 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-18 15:55 Volunteering to help on etc/TODO item: Improved xwidgets support Andrew De Angelis
2022-10-19  1:33 ` Po Lu
2022-10-19  3:43   ` Andrew De Angelis
2022-10-19  4:45     ` Po Lu
2022-10-19  4:55       ` Andrew De Angelis
2022-10-19  5:11         ` Po Lu
2022-10-19 18:54     ` Qiantan Hong
2022-10-20  2:23       ` Andrew De Angelis
2022-10-20  2:46         ` Po Lu
2022-10-20 11:13         ` Daniel Martín
2022-10-21 23:29           ` Andrew De Angelis
2022-10-22  0:07             ` Po Lu
2022-10-22  2:13               ` Andrew De Angelis
2022-10-22  4:49               ` Matt Armstrong

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).