unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code
       [not found] <871r3p8r05.fsf.ref@yahoo.com>
@ 2021-11-09 12:15 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2021-11-09 13:33   ` Eli Zaretskii
                     ` (2 more replies)
  0 siblings, 3 replies; 24+ messages in thread
From: Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-11-09 12:15 UTC (permalink / raw)
  To: 51716

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

I found it impossible to navigate the history of an Xwidget WebKit
buffer after following about a dozen links, and clicking back several
times.

This should lay the groundwork for a history navigator in Xwidget WebKit
buffers.  I will probably work on that tomorrow.

Thanks.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Expose-xwidget-navigation-history-to-Lisp-code.patch --]
[-- Type: text/x-patch, Size: 6824 bytes --]

From d487e2689212d3dab108e043d49a8aa1787a7015 Mon Sep 17 00:00:00 2001
From: Po Lu <luangruo@yahoo.com>
Date: Tue, 9 Nov 2021 20:11:05 +0800
Subject: [PATCH] Expose xwidget navigation history to Lisp code

* doc/lispref/display.texi (Xwidgets): Document changes.
* etc/NEWS: Announce new function.
* src/xwidget.c (Fxwidget_webkit_goto_history): Remove obsolete test.
(Fxwidget_webkit_back_forward_list): New function.
(syms_of_xwidget): Define new subr.
---
 doc/lispref/display.texi |  26 ++++++++++
 etc/NEWS                 |   5 ++
 src/xwidget.c            | 102 +++++++++++++++++++++++++++++++++++++--
 3 files changed, 130 insertions(+), 3 deletions(-)

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index 3ccc755391..a953162ae9 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -6953,6 +6953,32 @@ Xwidgets
 @var{base-uri}, which defaults to @samp{about:blank}.
 @end defun
 
+@defun xwidget-webkit-goto-history xwidget rel-pos
+Make @var{xwidget}, a WebKit widget, load the @code{rel-pos}th element
+in its navigation history.
+
+If @var{rel-pos} is zero, the current page will be reloaded instead.
+@end defun
+
+@defun xwidget-webkit-back-forward-list xwidget &optional limit
+Return the navigation history of @var{xwidget}, up to @var{limit}
+items in each direction.  If not specified, @var{limit} defaults to
+50.
+
+The returned value is a list of the form @code{(@var{back} @var{here}
+@var{forward})}, where @var{here} is the current navigation item,
+while @var{back} is a list of items containing the history behind the
+current navigation item, and @var{forward} is a list of items in front
+of the current navigation item.  @var{back}, @var{here} and
+@var{forward} can all be @code{nil}.
+
+Navigation items are themselves lists of the form @code{(@var{idx}
+@var{title} @var{uri})}.  In these lists, @var{idx} is an index that
+can be passed to @code{xwidget-webkit-goto-history}, @var{title} is
+the human-readable title of the item, and @var{uri} is the URI of the
+item.
+@end defun
+
 @node Buttons
 @section Buttons
 @cindex buttons in buffers
diff --git a/etc/NEWS b/etc/NEWS
index 93c161025b..82e8aff1b4 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -797,6 +797,11 @@ On GTK+, only key and function key events are implemented.
 This function is used to load HTML text into WebKit xwidgets, without
 having to create a temporary file or URL.
 
++++
+*** New function 'xwidget-webkit-back-forward-list'.
+This function is used to obtain the history of page-loads in a given
+WebKit xwidget.
+
 +++
 *** New functions for performing searches on WebKit xwidgets.
 Some new functions, such as 'xwidget-webkit-search', have been added
diff --git a/src/xwidget.c b/src/xwidget.c
index 22893bfe2e..c731f847c3 100644
--- a/src/xwidget.c
+++ b/src/xwidget.c
@@ -1617,9 +1617,7 @@ DEFUN ("xwidget-webkit-goto-history",
   (Lisp_Object xwidget, Lisp_Object rel_pos)
 {
   WEBKIT_FN_INIT ();
-  /* Should be one of -1, 0, 1 */
-  if (XFIXNUM (rel_pos) < -1 || XFIXNUM (rel_pos) > 1)
-    args_out_of_range_3 (rel_pos, make_fixnum (-1), make_fixnum (1));
+  CHECK_FIXNUM (rel_pos);
 
 #ifdef USE_GTK
   WebKitWebView *wkwv = WEBKIT_WEB_VIEW (xw->widget_osr);
@@ -2176,6 +2174,103 @@ DEFUN ("xwidget-webkit-load-html", Fxwidget_webkit_load_html,
   return Qnil;
 }
 
+DEFUN ("xwidget-webkit-back-forward-list", Fxwidget_webkit_back_forward_list,
+       Sxwidget_webkit_back_forward_list, 1, 2, 0,
+       doc: /* Return the navigation history of XWIDGET, a WebKit xwidget.
+
+The history is returned as a list of the form (BACK HERE FORWARD),
+where HERE is the current navigation item, while BACK and FORWARD are
+lists of navigation items, which are lists of the form (IDX TITLE
+URI).  Here, IDX is an index that can be passed to
+`xwidget-webkit-goto-history', TITLE is a string containing the
+human-readable title of the history item, and URI is the URI of the
+history item.  The user should normally have no reason to load URI
+manually to reach a specific history item.  Instead, he should pass
+IDX as an index to `xwidget-webkit-goto-history'.
+
+BACK, HERE, and FORWARD can all be nil if there are no items in the
+navigation history.
+
+BACK and FORWARD will not contain more elements than LIMIT.  If LIMIT
+is not specified or nil, it is treated as `50'.  */)
+  (Lisp_Object xwidget, Lisp_Object limit)
+{
+  struct xwidget *xw;
+  Lisp_Object back, here, forward;
+#ifdef USE_GTK
+  WebKitWebView *webview;
+  WebKitBackForwardList *list;
+  WebKitBackForwardListItem *item;
+  GList *parent, *tem;
+  int i;
+  unsigned int lim;
+  Lisp_Object title, uri;
+  const gchar *item_title, *item_uri;
+#endif
+
+  back = Qnil;
+  here = Qnil;
+  forward = Qnil;
+
+  if (NILP (limit))
+    limit = make_fixnum (50);
+
+  CHECK_FIXNAT (limit);
+  CHECK_XWIDGET (xwidget);
+  xw = XXWIDGET (xwidget);
+
+#ifdef USE_GTK
+  webview = WEBKIT_WEB_VIEW (xw->widget_osr);
+  list = webkit_web_view_get_back_forward_list (webview);
+  item = webkit_back_forward_list_get_current_item (list);
+  lim = XFIXNAT (limit);
+
+  if (item)
+    {
+      item_title = webkit_back_forward_list_item_get_title (item);
+      item_uri = webkit_back_forward_list_item_get_uri (item);
+      here = list3 (make_fixnum (0),
+		    build_string (item_title ? item_title : ""),
+		    build_string (item_uri ? item_uri : ""));
+    }
+  parent = webkit_back_forward_list_get_back_list_with_limit (list, lim);
+
+  if (parent)
+    {
+      for (i = 1, tem = parent; parent; parent = parent->next, ++i)
+	{
+	  item = tem->data;
+	  item_title = webkit_back_forward_list_item_get_title (item);
+	  item_uri = webkit_back_forward_list_item_get_uri (item);
+	  title = build_string (item_title ? item_title : "");
+	  uri = build_string (item_uri ? item_uri : "");
+	  back = Fcons (list3 (make_fixnum (-i), title, uri), back);
+	}
+    }
+
+  g_list_free (parent);
+
+  parent = webkit_back_forward_list_get_forward_list_with_limit (list, lim);
+
+  if (parent)
+    {
+      for (i = 1, tem = parent; parent; parent = parent->next, ++i)
+	{
+	  item = tem->data;
+	  item_title = webkit_back_forward_list_item_get_title (item);
+	  item_uri = webkit_back_forward_list_item_get_uri (item);
+	  title = build_string (item_title ? item_title : "");
+	  uri = build_string (item_uri ? item_uri : "");
+	  forward = Fcons (list3 (make_fixnum (i), title, uri), forward);
+	}
+    }
+
+  g_list_free (parent);
+#endif
+
+  return list3 (back, here, forward);
+}
+
 void
 syms_of_xwidget (void)
 {
@@ -2215,6 +2310,7 @@ syms_of_xwidget (void)
   defsubr (&Sxwidget_webkit_previous_result);
   defsubr (&Sset_xwidget_buffer);
   defsubr (&Sxwidget_webkit_load_html);
+  defsubr (&Sxwidget_webkit_back_forward_list);
 
   DEFSYM (QCxwidget, ":xwidget");
   DEFSYM (QCtitle, ":title");
-- 
2.31.1


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

* bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code
  2021-11-09 12:15 ` bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-11-09 13:33   ` Eli Zaretskii
  2021-11-09 13:44     ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2021-11-10 15:25   ` Eli Zaretskii
  2021-11-14 13:29   ` Po Lu
  2 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2021-11-09 13:33 UTC (permalink / raw)
  To: Po Lu; +Cc: 51716

> Date: Tue, 09 Nov 2021 20:15:06 +0800
> From:  Po Lu via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> I found it impossible to navigate the history of an Xwidget WebKit
> buffer after following about a dozen links, and clicking back several
> times.
> 
> This should lay the groundwork for a history navigator in Xwidget WebKit
> buffers.  I will probably work on that tomorrow.
> 
> Thanks.
> 
> >From d487e2689212d3dab108e043d49a8aa1787a7015 Mon Sep 17 00:00:00 2001
> From: Po Lu <luangruo@yahoo.com>
> Date: Tue, 9 Nov 2021 20:11:05 +0800
> Subject: [PATCH] Expose xwidget navigation history to Lisp code
> 
> * doc/lispref/display.texi (Xwidgets): Document changes.
> * etc/NEWS: Announce new function.
> * src/xwidget.c (Fxwidget_webkit_goto_history): Remove obsolete test.
> (Fxwidget_webkit_back_forward_list): New function.
> (syms_of_xwidget): Define new subr.
> ---
>  doc/lispref/display.texi |  26 ++++++++++
>  etc/NEWS                 |   5 ++
>  src/xwidget.c            | 102 +++++++++++++++++++++++++++++++++++++--
>  3 files changed, 130 insertions(+), 3 deletions(-)
> 
> diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
> index 3ccc755391..a953162ae9 100644
> --- a/doc/lispref/display.texi
> +++ b/doc/lispref/display.texi
> @@ -6953,6 +6953,32 @@ Xwidgets
>  @var{base-uri}, which defaults to @samp{about:blank}.
>  @end defun
>  
> +@defun xwidget-webkit-goto-history xwidget rel-pos
> +Make @var{xwidget}, a WebKit widget, load the @code{rel-pos}th element
> +in its navigation history.
> +
> +If @var{rel-pos} is zero, the current page will be reloaded instead.
> +@end defun
> +
> +@defun xwidget-webkit-back-forward-list xwidget &optional limit
> +Return the navigation history of @var{xwidget}, up to @var{limit}
> +items in each direction.  If not specified, @var{limit} defaults to
> +50.
> +
> +The returned value is a list of the form @code{(@var{back} @var{here}
> +@var{forward})}, where @var{here} is the current navigation item,
> +while @var{back} is a list of items containing the history behind the
> +current navigation item, and @var{forward} is a list of items in front
> +of the current navigation item.  @var{back}, @var{here} and
> +@var{forward} can all be @code{nil}.

Before I dive into the code, let me ask: why not use the usual Emacs
history machinery for this?  IOW, have some xwidget-specific history
variable that would store the browsing history, and use that variable
in conjunction with foo-prev and foo-next commands?  Why do we need to
reinvent the wheel for xwidgets, and why do that in C?





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

* bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code
  2021-11-09 13:33   ` Eli Zaretskii
@ 2021-11-09 13:44     ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2021-11-09 13:58       ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-11-09 13:44 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 51716

Eli Zaretskii <eliz@gnu.org> writes:

> Before I dive into the code, let me ask: why not use the usual Emacs
> history machinery for this?  IOW, have some xwidget-specific history
> variable that would store the browsing history, and use that variable
> in conjunction with foo-prev and foo-next commands?  Why do we need to
> reinvent the wheel for xwidgets, and why do that in C?

WebKit stores navigation history specially.  For instance, it implements
a feature where malicious (i.e. looping) redirects are not recorded in
history.  This is not available from Lisp, as the signals exposed by
WebKitGTK are insufficient to keep track of that data.  Further, I think
there is a way for web pages to tell WebKitGTK whether or not to record
themselves in history, which also cannot be implemented separately.

It can only be done from C, because the necessary information is only
available in C.





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

* bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code
  2021-11-09 13:44     ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-11-09 13:58       ` Eli Zaretskii
  2021-11-09 14:06         ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2021-11-09 13:58 UTC (permalink / raw)
  To: Po Lu; +Cc: 51716

> From: Po Lu <luangruo@yahoo.com>
> Cc: 51716@debbugs.gnu.org
> Date: Tue, 09 Nov 2021 21:44:47 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Before I dive into the code, let me ask: why not use the usual Emacs
> > history machinery for this?  IOW, have some xwidget-specific history
> > variable that would store the browsing history, and use that variable
> > in conjunction with foo-prev and foo-next commands?  Why do we need to
> > reinvent the wheel for xwidgets, and why do that in C?
> 
> WebKit stores navigation history specially.  For instance, it implements
> a feature where malicious (i.e. looping) redirects are not recorded in
> history.  This is not available from Lisp, as the signals exposed by
> WebKitGTK are insufficient to keep track of that data.  Further, I think
> there is a way for web pages to tell WebKitGTK whether or not to record
> themselves in history, which also cannot be implemented separately.
> 
> It can only be done from C, because the necessary information is only
> available in C.

Please give more details, because I still have only a very vague idea
of the problems you mention, and why they absolutely preclude mapping
the browsing history into Lisp.





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

* bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code
  2021-11-09 13:58       ` Eli Zaretskii
@ 2021-11-09 14:06         ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2021-11-09 14:10           ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-11-09 14:06 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 51716

Eli Zaretskii <eliz@gnu.org> writes:

>> WebKit stores navigation history specially.  For instance, it implements
>> a feature where malicious (i.e. looping) redirects are not recorded in
>> history.  This is not available from Lisp, as the signals exposed by
>> WebKitGTK are insufficient to keep track of that data.  Further, I think
>> there is a way for web pages to tell WebKitGTK whether or not to record
>> themselves in history, which also cannot be implemented separately.
>> 
>> It can only be done from C, because the necessary information is only
>> available in C.

> Please give more details, because I still have only a very vague idea
> of the problems you mention, and why they absolutely preclude mapping
> the browsing history into Lisp.

OK, I should clarify: this function provides the data necessary to map
browsing history into Lisp.

One feature I mentioned was that WebKit removes repetitive (usually
malicious) redirects designed to make it impossible to leave a page
through going back in the history.

WebKit only records its canonical page history in the back-forward list
of the WebKitWebView, which can only be obtained with
webkit_web_view_get_back_forward_list, and as such it needs to be
introduced as a primitive.

Thanks.






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

* bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code
  2021-11-09 14:06         ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-11-09 14:10           ` Eli Zaretskii
  2021-11-09 23:57             ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2021-11-09 14:10 UTC (permalink / raw)
  To: Po Lu; +Cc: 51716

> From: Po Lu <luangruo@yahoo.com>
> Cc: 51716@debbugs.gnu.org
> Date: Tue, 09 Nov 2021 22:06:45 +0800
> 
> OK, I should clarify: this function provides the data necessary to map
> browsing history into Lisp.

Yes, I've seen that.

> One feature I mentioned was that WebKit removes repetitive (usually
> malicious) redirects designed to make it impossible to leave a page
> through going back in the history.

And we cannot detect those repetitive redirects in Lisp?

> WebKit only records its canonical page history in the back-forward list
> of the WebKitWebView, which can only be obtained with
> webkit_web_view_get_back_forward_list, and as such it needs to be
> introduced as a primitive.

So you must rely on WebKit for producing the history?

But once the history is produced/updated, can it be treated as any
other history variable in Emacs?





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

* bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code
  2021-11-09 14:10           ` Eli Zaretskii
@ 2021-11-09 23:57             ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 0 replies; 24+ messages in thread
From: Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-11-09 23:57 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 51716

Eli Zaretskii <eliz@gnu.org> writes:

> And we cannot detect those repetitive redirects in Lisp?

Yes, and there are various other shenanigans that vary by system
configuration, WebKit configuration, WebKit version, security policy and
such that are impractical to keep track of.  I can't give a concrete
list though.

> So you must rely on WebKit for producing the history?

Yes.

> But once the history is produced/updated, can it be treated as any
> other history variable in Emacs?

Also correct, thanks.





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

* bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code
  2021-11-09 12:15 ` bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2021-11-09 13:33   ` Eli Zaretskii
@ 2021-11-10 15:25   ` Eli Zaretskii
  2021-11-11  1:03     ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2021-11-14 13:29   ` Po Lu
  2 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2021-11-10 15:25 UTC (permalink / raw)
  To: Po Lu; +Cc: 51716

> Date: Tue, 09 Nov 2021 20:15:06 +0800
> From:  Po Lu via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> +@defun xwidget-webkit-goto-history xwidget rel-pos
> +Make @var{xwidget}, a WebKit widget, load the @code{rel-pos}th element
                                                 ^^^^^
This should be @var, not @code.

> +in its navigation history.
> +
> +If @var{rel-pos} is zero, the current page will be reloaded instead.
> +@end defun
> +
> +@defun xwidget-webkit-back-forward-list xwidget &optional limit

Hmm... given that we have xwidget-webkit-goto-history, why do we need
this new function as well?  Or why do we need both?

> +The returned value is a list of the form @code{(@var{back} @var{here}
> +@var{forward})}

This list should be wrapped in @w{..}, so that it is kept unbroken
between 2 lines.

> where @var{here} is the current navigation item,
> +while @var{back} is a list of items containing the history behind the
> +current navigation item, and @var{forward} is a list of items in front
> +of the current navigation item.

The notions of "behind" and "in front" are not well-defined in this
context.  The text should make more clear what does each one of those
mean.

>                                  @var{back}, @var{here} and
> +@var{forward} can all be @code{nil}.

What is the meaning of each one being nil?  The text leaves that
unsaid.

> +Navigation items are themselves lists of the form @code{(@var{idx}
> +@var{title} @var{uri})}.

Please use @w{..} here as well.  basically you should use it for any
form that has embedded whitespace, and can therefore be split between
two lines when Texinfo fills and justifies the text.

>                           In these lists, @var{idx} is an index that
> +can be passed to @code{xwidget-webkit-goto-history}, @var{title} is
> +the human-readable title of the item, and @var{uri} is the URI of the
> +item.

URI, not URL?

> +DEFUN ("xwidget-webkit-back-forward-list", Fxwidget_webkit_back_forward_list,
> +       Sxwidget_webkit_back_forward_list, 1, 2, 0,
> +       doc: /* Return the navigation history of XWIDGET, a WebKit xwidget.
> +
> +The history is returned as a list of the form (BACK HERE FORWARD),
   ^^^^^^^^^^^^^^^^^^^^^^^
Passive tense alert!

> +where HERE is the current navigation item, while BACK and FORWARD are
> +lists of navigation items, which are lists of the form (IDX TITLE

"lists of ... , which are lists" uses "lists" twice, which is
redundant.  Better say

   ... are lists of elements of the form (IDX TITLE URI) ...

> +               The user should normally have no reason to load URI
> +manually to reach a specific history item.  Instead, he should pass
> +IDX as an index to `xwidget-webkit-goto-history'.

This belongs to the manual, not a doc string.  (And try to rephrase to
avoid using "he".)

> +BACK and FORWARD will not contain more elements than LIMIT.

Each one or both together? the text is ambiguous about that.

> +#ifdef USE_GTK
> +  webview = WEBKIT_WEB_VIEW (xw->widget_osr);

I guess this again means this function is a no-op without GTK?  Then
let's not define it except in the GTK build.

> +  list = webkit_web_view_get_back_forward_list (webview);
> +  item = webkit_back_forward_list_get_current_item (list);
> +  lim = XFIXNAT (limit);
> +
> +  if (item)
> +    {
> +      item_title = webkit_back_forward_list_item_get_title (item);
> +      item_uri = webkit_back_forward_list_item_get_uri (item);
> +      here = list3 (make_fixnum (0),
> +		    build_string (item_title ? item_title : ""),
> +		    build_string (item_uri ? item_uri : ""));
> +    }
> +  parent = webkit_back_forward_list_get_back_list_with_limit (list, lim);
> +
> +  if (parent)
> +    {
> +      for (i = 1, tem = parent; parent; parent = parent->next, ++i)
> +	{
> +	  item = tem->data;
> +	  item_title = webkit_back_forward_list_item_get_title (item);
> +	  item_uri = webkit_back_forward_list_item_get_uri (item);
> +	  title = build_string (item_title ? item_title : "");
> +	  uri = build_string (item_uri ? item_uri : "");

build_string can produce either multibyte or unibyte strings.  Which
ones do we want?  And shouldn't we decode the strings that come from
WebKit?

> +	  back = Fcons (list3 (make_fixnum (-i), title, uri), back);
> +	}
> +    }
> +
> +  g_list_free (parent);
> +
> +  parent = webkit_back_forward_list_get_forward_list_with_limit (list, lim);
> +
> +  if (parent)
> +    {
> +      for (i = 1, tem = parent; parent; parent = parent->next, ++i)
> +	{
> +	  item = tem->data;
> +	  item_title = webkit_back_forward_list_item_get_title (item);
> +	  item_uri = webkit_back_forward_list_item_get_uri (item);
> +	  title = build_string (item_title ? item_title : "");
> +	  uri = build_string (item_uri ? item_uri : "");
> +	  forward = Fcons (list3 (make_fixnum (i), title, uri), forward);
> +	}

This generates the lists in the reverse order, doesn't it?

Thanks.





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

* bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code
  2021-11-10 15:25   ` Eli Zaretskii
@ 2021-11-11  1:03     ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2021-11-13 15:02       ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-11-11  1:03 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 51716

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

Eli Zaretskii <eliz@gnu.org> writes:

> This should be @var, not @code.

>> +in its navigation history.
>> +
>> +If @var{rel-pos} is zero, the current page will be reloaded instead.
>> +@end defun
>> +
>> +@defun xwidget-webkit-back-forward-list xwidget &optional limit
>
> Hmm... given that we have xwidget-webkit-goto-history, why do we need
> this new function as well?  Or why do we need both?

`xwidget-webkit-goto-history' only lets you navigate to a history index:
it doesn't tell you what that index actually is, or indeed, if it even
exists at all.

>> +The returned value is a list of the form @code{(@var{back} @var{here}
>> +@var{forward})}

> This list should be wrapped in @w{..}, so that it is kept unbroken
> between 2 lines.

Thanks.

>> where @var{here} is the current navigation item,
>> +while @var{back} is a list of items containing the history behind the
>> +current navigation item, and @var{forward} is a list of items in front
>> +of the current navigation item.
>
> The notions of "behind" and "in front" are not well-defined in this
> context.  The text should make more clear what does each one of those
> mean.

I tried to clarify, thanks.

>>                                  @var{back}, @var{here} and
>> +@var{forward} can all be @code{nil}.

> What is the meaning of each one being nil?  The text leaves that
> unsaid.

BACK being nil means that we are at the beginning of the history.
FORWARD being nil means that there is nothing after the current place in
history.  HERE being nil means that WebKit hasn't recorded any pages in
history yet.

I will add that to the documentation, thanks.

> Please use @w{..} here as well.  basically you should use it for any
> form that has embedded whitespace, and can therefore be split between
> two lines when Texinfo fills and justifies the text.

Done, thanks.

>>                           In these lists, @var{idx} is an index that
>> +can be passed to @code{xwidget-webkit-goto-history}, @var{title} is
>> +the human-readable title of the item, and @var{uri} is the URI of the
>> +item.

> URI, not URL?

Yes, as WebKit allows any URI to appear in history items.

>> +DEFUN ("xwidget-webkit-back-forward-list", Fxwidget_webkit_back_forward_list,
>> +       Sxwidget_webkit_back_forward_list, 1, 2, 0,
>> +       doc: /* Return the navigation history of XWIDGET, a WebKit xwidget.
>> +
>> +The history is returned as a list of the form (BACK HERE FORWARD),
>    ^^^^^^^^^^^^^^^^^^^^^^^
> Passive tense alert!

[...]

> "lists of ... , which are lists" uses "lists" twice, which is
> redundant.  Better say
>
>    ... are lists of elements of the form (IDX TITLE URI) ...

Thanks, I fixed.

> This belongs to the manual, not a doc string.

Thanks, I moved it there.

> Each one or both together? the text is ambiguous about that.

I tried to fix that, please check.  Thanks.

>> +#ifdef USE_GTK
>> +  webview = WEBKIT_WEB_VIEW (xw->widget_osr);
>
> I guess this again means this function is a no-op without GTK?  Then
> let's not define it except in the GTK build.

Done, thanks.

>> +  list = webkit_web_view_get_back_forward_list (webview);
>> +  item = webkit_back_forward_list_get_current_item (list);
>> +  lim = XFIXNAT (limit);
>> +
>> +  if (item)
>> +    {
>> +      item_title = webkit_back_forward_list_item_get_title (item);
>> +      item_uri = webkit_back_forward_list_item_get_uri (item);
>> +      here = list3 (make_fixnum (0),
>> +		    build_string (item_title ? item_title : ""),
>> +		    build_string (item_uri ? item_uri : ""));
>> +    }
>> +  parent = webkit_back_forward_list_get_back_list_with_limit (list, lim);
>> +
>> +  if (parent)
>> +    {
>> +      for (i = 1, tem = parent; parent; parent = parent->next, ++i)
>> +	{
>> +	  item = tem->data;
>> +	  item_title = webkit_back_forward_list_item_get_title (item);
>> +	  item_uri = webkit_back_forward_list_item_get_uri (item);
>> +	  title = build_string (item_title ? item_title : "");
>> +	  uri = build_string (item_uri ? item_uri : "");

> build_string can produce either multibyte or unibyte strings.  Which
> ones do we want?  And shouldn't we decode the strings that come from
> WebKit?

Sorry for that. We want UTF-8 strings, as that's the format of strings
which come from WebKitGTK.  (And most GLib-related software, as well).

>> +	  back = Fcons (list3 (make_fixnum (-i), title, uri), back);
>> +	}
>> +    }
>> +
>> +  g_list_free (parent);
>> +
>> +  parent = webkit_back_forward_list_get_forward_list_with_limit (list, lim);
>> +
>> +  if (parent)
>> +    {
>> +      for (i = 1, tem = parent; parent; parent = parent->next, ++i)
>> +	{
>> +	  item = tem->data;
>> +	  item_title = webkit_back_forward_list_item_get_title (item);
>> +	  item_uri = webkit_back_forward_list_item_get_uri (item);
>> +	  title = build_string (item_title ? item_title : "");
>> +	  uri = build_string (item_uri ? item_uri : "");
>> +	  forward = Fcons (list3 (make_fixnum (i), title, uri), forward);
>> +	}

> This generates the lists in the reverse order, doesn't it?

Yes, but what's important here is the index `i', not the order of the
list.  But I agree that it would be OK to nreverse the contents of that
list.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Expose-xwidget-navigation-history-to-Lisp-code.patch --]
[-- Type: text/x-patch, Size: 6794 bytes --]

From 920ae609bb5fbeab4bb9511fd5539978c10c4448 Mon Sep 17 00:00:00 2001
From: Po Lu <luangruo@yahoo.com>
Date: Thu, 11 Nov 2021 09:01:38 +0800
Subject: [PATCH] Expose xwidget navigation history to Lisp code

* doc/lispref/display.texi (Xwidgets): Document changes.
* etc/NEWS: Announce new function.
* src/xwidget.c (Fxwidget_webkit_back_forward_list): New function.
(syms_of_xwidget): Define new subr.
---
 doc/lispref/display.texi | 33 ++++++++++++++
 etc/NEWS                 |  5 +++
 src/xwidget.c            | 95 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 133 insertions(+)

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index ad1077e0c4..094bb5a5f3 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -6953,6 +6953,39 @@ Xwidgets
 to be used for resolving relative links in @var{text}.
 @end defun
 
+@defun xwidget-webkit-goto-history xwidget rel-pos
+Make @var{xwidget}, a WebKit widget, load the @var{rel-pos}th element
+in its navigation history.
+
+If @var{rel-pos} is zero, the current page will be reloaded instead.
+@end defun
+
+@defun xwidget-webkit-back-forward-list xwidget &optional limit
+Return the navigation history of @var{xwidget}, up to @var{limit}
+items in each direction.  If not specified, @var{limit} defaults to
+50.
+
+The returned value is a list of the form @w{@code{(@var{back}
+@var{here} @var{forward})}}, where @var{here} is the current
+navigation item, while @var{back} is a list of items containing the
+items recorded by WebKit before the current navigation item, and
+@var{forward} is a list of items recorded after the current navigation
+item.  @var{back}, @var{here} and @var{forward} can all be @code{nil}.
+
+When @var{here} is @code{nil}, it means that no items have been
+recorded yet; if @var{back} or @var{forward} are @code{nil}, it means
+that there is no history recorded before or after the current item
+respectively.
+
+Navigation items are themselves lists of the form @w{@code{(@var{idx}
+@var{title} @var{uri})}}.  In these lists, @var{idx} is an index that
+can be passed to @code{xwidget-webkit-goto-history}, @var{title} is
+the human-readable title of the item, and @var{uri} is the URI of the
+item.  The user should normally have no reason to load @var{uri}
+manually to reach a specific history item.  Instead, @var{idx} should
+be passed as an index to @code{xwidget-webkit-goto-history}.
+@end defun
+
 @node Buttons
 @section Buttons
 @cindex buttons in buffers
diff --git a/etc/NEWS b/etc/NEWS
index 78ce3c067f..7702cec92c 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -747,6 +747,11 @@ markup, and passing the URI of the file as an argument to
 Some new functions, such as 'xwidget-webkit-search', have been added
 for performing searches on WebKit xwidgets.
 
++++
+*** New function 'xwidget-webkit-back-forward-list'.
+This function is used to obtain the history of page-loads in a given
+WebKit xwidget.
+
 +++
 *** 'load-changed' xwidget events are now more detailed.
 In particular, they can now have different arguments based on the
diff --git a/src/xwidget.c b/src/xwidget.c
index fc05f4f570..ace63e3742 100644
--- a/src/xwidget.c
+++ b/src/xwidget.c
@@ -20,6 +20,7 @@ Copyright (C) 2011-2021 Free Software Foundation, Inc.
 #include <config.h>
 
 #include "buffer.h"
+#include "coding.h"
 #include "xwidget.h"
 
 #include "lisp.h"
@@ -2314,6 +2315,99 @@ DEFUN ("xwidget-webkit-load-html", Fxwidget_webkit_load_html,
 
   return Qnil;
 }
+
+DEFUN ("xwidget-webkit-back-forward-list", Fxwidget_webkit_back_forward_list,
+       Sxwidget_webkit_back_forward_list, 1, 2, 0,
+       doc: /* Return the navigation history of XWIDGET, a WebKit xwidget.
+
+Return the history as a list of the form (BACK HERE FORWARD), where
+HERE is the current navigation item, while BACK and FORWARD are lists
+of history items of the form (IDX TITLE URI).  Here, IDX is an index
+that can be passed to `xwidget-webkit-goto-history', TITLE is a string
+containing the human-readable title of the history item, and URI is
+the URI of the history item.
+
+BACK, HERE, and FORWARD can all be nil depending on the state of the
+navigation history.
+
+BACK and FORWARD will each not contain more elements than LIMIT.  If
+LIMIT is not specified or nil, it is treated as `50'.  */)
+  (Lisp_Object xwidget, Lisp_Object limit)
+{
+  struct xwidget *xw;
+  Lisp_Object back, here, forward;
+  WebKitWebView *webview;
+  WebKitBackForwardList *list;
+  WebKitBackForwardListItem *item;
+  GList *parent, *tem;
+  int i;
+  unsigned int lim;
+  Lisp_Object title, uri;
+  const gchar *item_title, *item_uri;
+
+  back = Qnil;
+  here = Qnil;
+  forward = Qnil;
+
+  if (NILP (limit))
+    limit = make_fixnum (50);
+  else
+    CHECK_FIXNAT (limit);
+
+  CHECK_XWIDGET (xwidget);
+  xw = XXWIDGET (xwidget);
+
+  webview = WEBKIT_WEB_VIEW (xw->widget_osr);
+  list = webkit_web_view_get_back_forward_list (webview);
+  item = webkit_back_forward_list_get_current_item (list);
+  lim = XFIXNAT (limit);
+
+  if (item)
+    {
+      item_title = webkit_back_forward_list_item_get_title (item);
+      item_uri = webkit_back_forward_list_item_get_uri (item);
+      here = list3 (make_fixnum (0),
+		    build_string_from_utf8 (item_title ? item_title : ""),
+		    build_string_from_utf8 (item_uri ? item_uri : ""));
+    }
+  parent = webkit_back_forward_list_get_back_list_with_limit (list, lim);
+
+  if (parent)
+    {
+      for (i = 1, tem = parent; parent; parent = parent->next, ++i)
+	{
+	  item = tem->data;
+	  item_title = webkit_back_forward_list_item_get_title (item);
+	  item_uri = webkit_back_forward_list_item_get_uri (item);
+	  title = build_string_from_utf8 (item_title ? item_title : "");
+	  uri = build_string_from_utf8 (item_uri ? item_uri : "");
+	  back = Fcons (list3 (make_fixnum (-i), title, uri), back);
+	}
+    }
+
+  back = Fnreverse (back);
+  g_list_free (parent);
+
+  parent = webkit_back_forward_list_get_forward_list_with_limit (list, lim);
+
+  if (parent)
+    {
+      for (i = 1, tem = parent; parent; parent = parent->next, ++i)
+	{
+	  item = tem->data;
+	  item_title = webkit_back_forward_list_item_get_title (item);
+	  item_uri = webkit_back_forward_list_item_get_uri (item);
+	  title = build_string_from_utf8 (item_title ? item_title : "");
+	  uri = build_string_from_utf8 (item_uri ? item_uri : "");
+	  forward = Fcons (list3 (make_fixnum (i), title, uri), forward);
+	}
+    }
+
+  forward = Fnreverse (forward);
+  g_list_free (parent);
+
+  return list3 (back, here, forward);
+}
 #endif
 
 void
@@ -2356,6 +2450,7 @@ syms_of_xwidget (void)
   defsubr (&Sset_xwidget_buffer);
 #ifdef USE_GTK
   defsubr (&Sxwidget_webkit_load_html);
+  defsubr (&Sxwidget_webkit_back_forward_list);
 #endif
 
   DEFSYM (QCxwidget, ":xwidget");
-- 
2.31.1


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

* bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code
  2021-11-11  1:03     ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-11-13 15:02       ` Eli Zaretskii
  2021-11-14  0:18         ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2021-11-13 15:02 UTC (permalink / raw)
  To: Po Lu; +Cc: 51716

> From: Po Lu <luangruo@yahoo.com>
> Cc: 51716@debbugs.gnu.org
> Date: Thu, 11 Nov 2021 09:03:19 +0800
> 
> >> +  if (item)
> >> +    {
> >> +      item_title = webkit_back_forward_list_item_get_title (item);
> >> +      item_uri = webkit_back_forward_list_item_get_uri (item);
> >> +      here = list3 (make_fixnum (0),
> >> +		    build_string (item_title ? item_title : ""),
> >> +		    build_string (item_uri ? item_uri : ""));
> >> +    }
> >> +  parent = webkit_back_forward_list_get_back_list_with_limit (list, lim);
> >> +
> >> +  if (parent)
> >> +    {
> >> +      for (i = 1, tem = parent; parent; parent = parent->next, ++i)
> >> +	{
> >> +	  item = tem->data;
> >> +	  item_title = webkit_back_forward_list_item_get_title (item);
> >> +	  item_uri = webkit_back_forward_list_item_get_uri (item);
> >> +	  title = build_string (item_title ? item_title : "");
> >> +	  uri = build_string (item_uri ? item_uri : "");
> 
> > build_string can produce either multibyte or unibyte strings.  Which
> > ones do we want?  And shouldn't we decode the strings that come from
> > WebKit?
> 
> Sorry for that. We want UTF-8 strings, as that's the format of strings
> which come from WebKitGTK.  (And most GLib-related software, as well).

Sorry, I don't understand: the above produces strings that will be
used by Emacs Lisp.  So UTF-8 is not relevant; the question is whether
the string that comes from WebKitGTK can include non-ASCII characters.
If it can, we need to decode it, e.g. by decode_string_utf_8 (can
these strings include byte sequences that aren't valid UTF-8?).

When this issue is resolved, the changes can be installed.

Thanks.





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

* bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code
  2021-11-13 15:02       ` Eli Zaretskii
@ 2021-11-14  0:18         ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2021-11-14  6:53           ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-11-14  0:18 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 51716

Eli Zaretskii <eliz@gnu.org> writes:

> Sorry, I don't understand: the above produces strings that will be
> used by Emacs Lisp.  So UTF-8 is not relevant; the question is whether
> the string that comes from WebKitGTK can include non-ASCII characters.
> If it can, we need to decode it, e.g. by decode_string_utf_8 (can
> these strings include byte sequences that aren't valid UTF-8?).

Ah, they cannot include byte sequences that aren't valid UTF-8.  Do you
see any further problem?  If not, I will install it with minor changes
(replacing CHECK_XWIDGET with CHECK_LIVE_XWIDGET).

Thanks.





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

* bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code
  2021-11-14  0:18         ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-11-14  6:53           ` Eli Zaretskii
  2021-11-14  6:58             ` Lars Ingebrigtsen
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2021-11-14  6:53 UTC (permalink / raw)
  To: Po Lu; +Cc: 51716

> From: Po Lu <luangruo@yahoo.com>
> Cc: 51716@debbugs.gnu.org
> Date: Sun, 14 Nov 2021 08:18:39 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Sorry, I don't understand: the above produces strings that will be
> > used by Emacs Lisp.  So UTF-8 is not relevant; the question is whether
> > the string that comes from WebKitGTK can include non-ASCII characters.
> > If it can, we need to decode it, e.g. by decode_string_utf_8 (can
> > these strings include byte sequences that aren't valid UTF-8?).
> 
> Ah, they cannot include byte sequences that aren't valid UTF-8.

If they cannot include invalid UTF-8, then using decode_string_utf_8
is what you need to do.  You cannot use those strings directly in
Lisp, because they will be unibyte strings.  IOW, using build_string
there is incorrect, you need to use make_unibyte_string and
decode_string_utf_8 instead.  build_string is correct only for
plain-ASCII strings.





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

* bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code
  2021-11-14  6:53           ` Eli Zaretskii
@ 2021-11-14  6:58             ` Lars Ingebrigtsen
  2021-11-14  6:59               ` Lars Ingebrigtsen
  2021-11-14  7:20               ` Eli Zaretskii
  0 siblings, 2 replies; 24+ messages in thread
From: Lars Ingebrigtsen @ 2021-11-14  6:58 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Po Lu, 51716

Eli Zaretskii <eliz@gnu.org> writes:

> If they cannot include invalid UTF-8, then using decode_string_utf_8
> is what you need to do.  You cannot use those strings directly in
> Lisp, because they will be unibyte strings.  IOW, using build_string
> there is incorrect, you need to use make_unibyte_string and
> decode_string_utf_8 instead.  build_string is correct only for
> plain-ASCII strings.

Can't he use make_multibyte_string instead?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code
  2021-11-14  6:58             ` Lars Ingebrigtsen
@ 2021-11-14  6:59               ` Lars Ingebrigtsen
  2021-11-14  7:11                 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2021-11-14  7:20               ` Eli Zaretskii
  1 sibling, 1 reply; 24+ messages in thread
From: Lars Ingebrigtsen @ 2021-11-14  6:59 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Po Lu, 51716

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Can't he use make_multibyte_string instead?

Never mind -- the interface probably doesn't say how many characters
there are in the string, only how many bytes.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code
  2021-11-14  6:59               ` Lars Ingebrigtsen
@ 2021-11-14  7:11                 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2021-11-14  7:18                   ` Lars Ingebrigtsen
  2021-11-14  7:29                   ` Eli Zaretskii
  0 siblings, 2 replies; 24+ messages in thread
From: Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-11-14  7:11 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Eli Zaretskii, 51716

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Lars Ingebrigtsen <larsi@gnus.org> writes:
>
>> Can't he use make_multibyte_string instead?
>
> Never mind -- the interface probably doesn't say how many characters
> there are in the string, only how many bytes.

What about `build_string_from_utf8'?  It seems to call
`make_string_from_utf8' with the length of the string in bytes, which
should be correct.





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

* bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code
  2021-11-14  7:11                 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-11-14  7:18                   ` Lars Ingebrigtsen
  2021-11-14  7:20                     ` Lars Ingebrigtsen
  2021-11-14  7:21                     ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2021-11-14  7:29                   ` Eli Zaretskii
  1 sibling, 2 replies; 24+ messages in thread
From: Lars Ingebrigtsen @ 2021-11-14  7:18 UTC (permalink / raw)
  To: Po Lu; +Cc: 51716

Po Lu <luangruo@yahoo.com> writes:

> What about `build_string_from_utf8'?  It seems to call
> `make_string_from_utf8' with the length of the string in bytes, which
> should be correct.

Yes, that looks like the function to use (and basically does what Eli
said, but with extra sanity checks, if I'm reading it correctly).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code
  2021-11-14  6:58             ` Lars Ingebrigtsen
  2021-11-14  6:59               ` Lars Ingebrigtsen
@ 2021-11-14  7:20               ` Eli Zaretskii
  1 sibling, 0 replies; 24+ messages in thread
From: Eli Zaretskii @ 2021-11-14  7:20 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: luangruo, 51716

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Po Lu <luangruo@yahoo.com>,  51716@debbugs.gnu.org
> Date: Sun, 14 Nov 2021 07:58:11 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > If they cannot include invalid UTF-8, then using decode_string_utf_8
> > is what you need to do.  You cannot use those strings directly in
> > Lisp, because they will be unibyte strings.  IOW, using build_string
> > there is incorrect, you need to use make_unibyte_string and
> > decode_string_utf_8 instead.  build_string is correct only for
> > plain-ASCII strings.
> 
> Can't he use make_multibyte_string instead?

I'd rather not.  Code which uses external strings without decoding
looks wrong, and the time it takes to convince yourself it does TRT
(if it does) is time lost.

decode_string_utf_8 is very fast, and xwidgets aren't supposed to be
in the inner loops of Emacs.  So I see no reason for shortcuts here.





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

* bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code
  2021-11-14  7:18                   ` Lars Ingebrigtsen
@ 2021-11-14  7:20                     ` Lars Ingebrigtsen
  2021-11-14  7:21                     ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 0 replies; 24+ messages in thread
From: Lars Ingebrigtsen @ 2021-11-14  7:20 UTC (permalink / raw)
  To: Po Lu; +Cc: 51716

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Yes, that looks like the function to use (and basically does what Eli
> said, but with extra sanity checks, if I'm reading it correctly).

(And I didn't, because the checks are #if 0-ed.  😀)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code
  2021-11-14  7:18                   ` Lars Ingebrigtsen
  2021-11-14  7:20                     ` Lars Ingebrigtsen
@ 2021-11-14  7:21                     ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 0 replies; 24+ messages in thread
From: Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-11-14  7:21 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Eli Zaretskii, 51716

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Yes, that looks like the function to use (and basically does what Eli
> said, but with extra sanity checks, if I'm reading it correctly).

Thanks, if Eli thinks that's fine, then I will push the change with the
adjustments pointed out here made.





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

* bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code
  2021-11-14  7:11                 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2021-11-14  7:18                   ` Lars Ingebrigtsen
@ 2021-11-14  7:29                   ` Eli Zaretskii
  2021-11-14  7:33                     ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2021-11-14  7:29 UTC (permalink / raw)
  To: Po Lu; +Cc: larsi, 51716

> From: Po Lu <luangruo@yahoo.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  51716@debbugs.gnu.org
> Date: Sun, 14 Nov 2021 15:11:30 +0800
> 
> Lars Ingebrigtsen <larsi@gnus.org> writes:
> 
> > Lars Ingebrigtsen <larsi@gnus.org> writes:
> >
> >> Can't he use make_multibyte_string instead?
> >
> > Never mind -- the interface probably doesn't say how many characters
> > there are in the string, only how many bytes.
> 
> What about `build_string_from_utf8'?  It seems to call
> `make_string_from_utf8' with the length of the string in bytes, which
> should be correct.

You want to save the explicit call to strlen (otherwise there's no
difference between my suggestion and build_string_from_utf8)?  Sure,
but that's not the important point I was trying to make.





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

* bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code
  2021-11-14  7:29                   ` Eli Zaretskii
@ 2021-11-14  7:33                     ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2021-11-14  8:04                       ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-11-14  7:33 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: larsi, 51716

Eli Zaretskii <eliz@gnu.org> writes:

> You want to save the explicit call to strlen (otherwise there's no
> difference between my suggestion and build_string_from_utf8)?  Sure,
> but that's not the important point I was trying to make.

Yes, I understood your point.  I'll switch the calls to use
`build_string_from_utf8' and install the change, if that's fine with
you.

Thanks a lot!





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

* bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code
  2021-11-14  7:33                     ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-11-14  8:04                       ` Eli Zaretskii
  2021-11-14  9:45                         ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2021-11-14  8:04 UTC (permalink / raw)
  To: Po Lu; +Cc: larsi, 51716

> From: Po Lu <luangruo@yahoo.com>
> Cc: larsi@gnus.org,  51716@debbugs.gnu.org
> Date: Sun, 14 Nov 2021 15:33:01 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > You want to save the explicit call to strlen (otherwise there's no
> > difference between my suggestion and build_string_from_utf8)?  Sure,
> > but that's not the important point I was trying to make.
> 
> Yes, I understood your point.  I'll switch the calls to use
> `build_string_from_utf8' and install the change, if that's fine with
> you.

Yes, I said that decoding was the last issue I had with the patch.





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

* bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code
  2021-11-14  8:04                       ` Eli Zaretskii
@ 2021-11-14  9:45                         ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 0 replies; 24+ messages in thread
From: Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-11-14  9:45 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: larsi, 51716

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Po Lu <luangruo@yahoo.com>
>> Cc: larsi@gnus.org,  51716@debbugs.gnu.org
>> Date: Sun, 14 Nov 2021 15:33:01 +0800
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > You want to save the explicit call to strlen (otherwise there's no
>> > difference between my suggestion and build_string_from_utf8)?  Sure,
>> > but that's not the important point I was trying to make.
>> 
>> Yes, I understood your point.  I'll switch the calls to use
>> `build_string_from_utf8' and install the change, if that's fine with
>> you.
>
> Yes, I said that decoding was the last issue I had with the patch.

Thanks, pushed.





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

* bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code
  2021-11-09 12:15 ` bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2021-11-09 13:33   ` Eli Zaretskii
  2021-11-10 15:25   ` Eli Zaretskii
@ 2021-11-14 13:29   ` Po Lu
  2 siblings, 0 replies; 24+ messages in thread
From: Po Lu @ 2021-11-14 13:29 UTC (permalink / raw)
  To: 51716-done

Forgot to close the issue, so here it is.
Sorry.





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

end of thread, other threads:[~2021-11-14 13:29 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <871r3p8r05.fsf.ref@yahoo.com>
2021-11-09 12:15 ` bug#51716: 29.0.50; [PATCH] Expose xwidget navigation history to Lisp code Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-11-09 13:33   ` Eli Zaretskii
2021-11-09 13:44     ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-11-09 13:58       ` Eli Zaretskii
2021-11-09 14:06         ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-11-09 14:10           ` Eli Zaretskii
2021-11-09 23:57             ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-11-10 15:25   ` Eli Zaretskii
2021-11-11  1:03     ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-11-13 15:02       ` Eli Zaretskii
2021-11-14  0:18         ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-11-14  6:53           ` Eli Zaretskii
2021-11-14  6:58             ` Lars Ingebrigtsen
2021-11-14  6:59               ` Lars Ingebrigtsen
2021-11-14  7:11                 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-11-14  7:18                   ` Lars Ingebrigtsen
2021-11-14  7:20                     ` Lars Ingebrigtsen
2021-11-14  7:21                     ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-11-14  7:29                   ` Eli Zaretskii
2021-11-14  7:33                     ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-11-14  8:04                       ` Eli Zaretskii
2021-11-14  9:45                         ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-11-14  7:20               ` Eli Zaretskii
2021-11-14 13:29   ` Po Lu

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).