GNU bug report logs - #22465
xwidget-webkit-scroll-behaviour = image is non-functional

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 22465 in the body.
You can then email your comments to 22465 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #3 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: submit <at> debbugs.gnu.org
Subject: xwidget-webkit-scroll-behaviour = image is non-functional
Date: Mon, 25 Jan 2016 18:19:53 -0500

Package: emacs
Version: 25.0.50

When xwidget-webkit-scroll-behavior = image, the functions
xwidget-webkit-scroll-forward and xwidget-webkit-scroll-backward are
obviously broken (they just call themselves recursively forever).

Message #8 received at 22465 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 22465 <at> debbugs.gnu.org
Subject: Re: xwidget-webkit-scroll-behaviour = image is non-functional
Date: Thu, 5 May 2016 22:48:00 -0700

[Message part 1 (text/plain, inline)]

Attached is a proposed patch for this bug. It is not fancy, but it works for me.

[0001-Fix-xwidget-webkit-scroll-forward-infloop.fix (text/plain, attachment)]

Message #13 received at 22465 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 22465 <at> debbugs.gnu.org, Joakim Verona <joakim <at> verona.se>
Subject: Re: xwidget-webkit-scroll-behaviour = image is non-functional
Date: Fri, 06 May 2016 11:42:39 -0400

Paul Eggert wrote:

> Attached is a proposed patch for this bug. It is not fancy, but it
> works for me.

Thanks. I wonder if the 'image scrolling method should simply be
removed, since it doesn't work (to date) and isn't the default.
Are two methods really needed/useful?

> From 879895ad058294eb49d8372ef5dbf053973e45b9 Mon Sep 17 00:00:00 2001
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Thu, 5 May 2016 22:44:16 -0700
> Subject: [PATCH] Fix xwidget-webkit-scroll-forward infloop
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> * lisp/xwidget.el (xwidget-webkit-scroll-forward)
> (xwidget-webkit-scroll-backward): Don't loop if
> xwidget-webkit-scroll-behavior's value is 'image' (Bug#22465).
> ---
>  lisp/xwidget.el | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lisp/xwidget.el b/lisp/xwidget.el
> index 19f631f..d64237a 100644
> --- a/lisp/xwidget.el
> +++ b/lisp/xwidget.el
> @@ -165,7 +165,7 @@ xwidget-webkit-scroll-forward
>    (interactive)
>    (if (eq xwidget-webkit-scroll-behavior 'native)
>        (xwidget-set-adjustment (xwidget-webkit-last-session) 'horizontal t 50)
> -    (xwidget-webkit-scroll-forward)))   ; FIXME infloop!
> +    (image-forward-hscroll 5)))
>  
>  (defun xwidget-webkit-scroll-backward ()
>    "Scroll webkit backwards.
> @@ -174,7 +174,7 @@ xwidget-webkit-scroll-backward
>    (interactive)
>    (if (eq xwidget-webkit-scroll-behavior 'native)
>        (xwidget-set-adjustment (xwidget-webkit-last-session) 'horizontal t -50)
> -    (xwidget-webkit-scroll-backward))) ; FIXME infloop!
> +    (image-backward-hscroll 5)))
>  
>  
>  ;; The xwidget event needs to go into a higher level handler

Message #16 received at 22465 <at> debbugs.gnu.org (full text, mbox):

From: joakim <at> verona.se
To: Glenn Morris <rgm <at> gnu.org>
Cc: 22465 <at> debbugs.gnu.org, Paul Eggert <eggert <at> cs.ucla.edu>
Subject: Re: xwidget-webkit-scroll-behaviour = image is non-functional
Date: Fri, 06 May 2016 19:50:25 +0200

Glenn Morris <rgm <at> gnu.org> writes:

> Paul Eggert wrote:
>
>> Attached is a proposed patch for this bug. It is not fancy, but it
>> works for me.
>
> Thanks. I wonder if the 'image scrolling method should simply be
> removed, since it doesn't work (to date) and isn't the default.
> Are two methods really needed/useful?

Short version:

It is probably just as well to remove the 'image method.

Long version:

The 'image method was the original scrolling method, which simply makes
a large xwidget and scrolls it using the same method as emacs uses to
scroll images. I liked this method because it was consistent with other
Emacs scrolling.

The 'native method lets webkit do the scrolling.

Originally I used the 'image method myself and added the 'native method
because users wanted it. Now I never use 'image, only 'native.

Perhaps someone still prefers the 'image method though.



>
>> From 879895ad058294eb49d8372ef5dbf053973e45b9 Mon Sep 17 00:00:00 2001
>> From: Paul Eggert <eggert <at> cs.ucla.edu>
>> Date: Thu, 5 May 2016 22:44:16 -0700
>> Subject: [PATCH] Fix xwidget-webkit-scroll-forward infloop
>> MIME-Version: 1.0
>> Content-Type: text/plain; charset=UTF-8
>> Content-Transfer-Encoding: 8bit
>>
>> * lisp/xwidget.el (xwidget-webkit-scroll-forward)
>> (xwidget-webkit-scroll-backward): Don't loop if
>> xwidget-webkit-scroll-behavior's value is 'image' (Bug#22465).
>> ---
>>  lisp/xwidget.el | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/lisp/xwidget.el b/lisp/xwidget.el
>> index 19f631f..d64237a 100644
>> --- a/lisp/xwidget.el
>> +++ b/lisp/xwidget.el
>> @@ -165,7 +165,7 @@ xwidget-webkit-scroll-forward
>>    (interactive)
>>    (if (eq xwidget-webkit-scroll-behavior 'native)
>>        (xwidget-set-adjustment (xwidget-webkit-last-session) 'horizontal t 50)
>> -    (xwidget-webkit-scroll-forward)))   ; FIXME infloop!
>> +    (image-forward-hscroll 5)))
>>  
>>  (defun xwidget-webkit-scroll-backward ()
>>    "Scroll webkit backwards.
>> @@ -174,7 +174,7 @@ xwidget-webkit-scroll-backward
>>    (interactive)
>>    (if (eq xwidget-webkit-scroll-behavior 'native)
>>        (xwidget-set-adjustment (xwidget-webkit-last-session) 'horizontal t -50)
>> -    (xwidget-webkit-scroll-backward))) ; FIXME infloop!
>> +    (image-backward-hscroll 5)))
>>  
>>  
>>  ;; The xwidget event needs to go into a higher level handler

-- 
Joakim Verona

Message #19 received at 22465 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: joakim <at> verona.se, Glenn Morris <rgm <at> gnu.org>
Cc: 22465 <at> debbugs.gnu.org
Subject: Re: xwidget-webkit-scroll-behaviour = image is non-functional
Date: Sat, 7 May 2016 17:21:45 -0700

[Message part 1 (text/plain, inline)]

joakim <at> verona.se wrote:
> Perhaps someone still prefers the 'image method though.

This seems unlikely, given that it recurses infinitely whenever it scrolls left
or right. Anyway, thanks for following up. If nobody objects I would like to
install the attached patch to emacs-25 to remove xwidget-webkit-scroll-behavior
and therefore fix the bug.

[0001-Remove-buggy-non-native-image-scrolling.patch (text/x-diff, attachment)]

Message #24 received at 22465-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: joakim <at> verona.se
Cc: 22465-done <at> debbugs.gnu.org, Glenn Morris <rgm <at> gnu.org>
Subject: Re: xwidget-webkit-scroll-behaviour = image is non-functional
Date: Fri, 13 May 2016 09:53:40 -0700

On 05/13/2016 01:05 AM, joakim <at> verona.se wrote:
> I agree, please install this fix.

Thanks, installed, and closing the bug.

Message #27 received at 22465 <at> debbugs.gnu.org (full text, mbox):

From: joakim <at> verona.se
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 22465 <at> debbugs.gnu.org, Glenn Morris <rgm <at> gnu.org>
Subject: Re: xwidget-webkit-scroll-behaviour = image is non-functional
Date: Fri, 13 May 2016 10:05:16 +0200

Paul Eggert <eggert <at> cs.ucla.edu> writes:

> joakim <at> verona.se wrote:
>> Perhaps someone still prefers the 'image method though.
>
> This seems unlikely, given that it recurses infinitely whenever it scrolls left
> or right. Anyway, thanks for following up. If nobody objects I would like to
> install the attached patch to emacs-25 to remove xwidget-webkit-scroll-behavior
> and therefore fix the bug.

I agree, please install this fix.

>
> From 58b7ecc4bcba46c775a7edca209587f034185861 Mon Sep 17 00:00:00 2001
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Sat, 7 May 2016 17:17:55 -0700
> Subject: [PATCH] Remove buggy non-native image scrolling
>
> This never worked, and could cause infinite recursion.
> Problem reported by Glenn Morris (Bug#22465).
> * lisp/xwidget.el (xwidget-webkit-scroll-behavior): Remove.
> All uses removed.
> ---
>  lisp/xwidget.el | 43 +++++++++----------------------------------
>  1 file changed, 9 insertions(+), 34 deletions(-)
>
> diff --git a/lisp/xwidget.el b/lisp/xwidget.el
> index 19f631f..7a0ca8b 100644
> --- a/lisp/xwidget.el
> +++ b/lisp/xwidget.el
> @@ -34,13 +34,6 @@
>  (require 'cl-lib)
>  (require 'bookmark)
>  
> -(defcustom xwidget-webkit-scroll-behavior 'native
> -  "Scrolling behavior of the webkit instance.
> -The possible values are: `native' or `image'."
> -  :version "25.1"
> -  :group 'frames   ; TODO add xwidgets group if more options are added
> -  :type '(choice (const native) (const image)))
> -
>  (declare-function make-xwidget "xwidget.c"
>                    (type title width height arguments &optional buffer))
>  (declare-function xwidget-set-adjustment "xwidget.c"
> @@ -141,40 +134,24 @@ xwidget-webkit-mode-map
>    "Keymap for `xwidget-webkit-mode'.")
>  
>  (defun xwidget-webkit-scroll-up ()
> -  "Scroll webkit up.
> -Depending on the value of `xwidget-webkit-scroll-behavior',
> -this scrolls in `native' fashion, or like `image-mode' would."
> +  "Scroll webkit up."
>    (interactive)
> -  (if (eq xwidget-webkit-scroll-behavior 'native)
> -      (xwidget-set-adjustment (xwidget-webkit-last-session) 'vertical t 50)
> -    (image-scroll-up)))
> +  (xwidget-set-adjustment (xwidget-webkit-last-session) 'vertical t 50))
>  
>  (defun xwidget-webkit-scroll-down ()
> -  "Scroll webkit down.
> -Depending on the value of `xwidget-webkit-scroll-behavior',
> -this scrolls in `native' fashion, or like `image-mode' would."
> +  "Scroll webkit down."
>    (interactive)
> -  (if (eq xwidget-webkit-scroll-behavior 'native)
> -      (xwidget-set-adjustment (xwidget-webkit-last-session) 'vertical t -50)
> -    (image-scroll-down)))
> +  (xwidget-set-adjustment (xwidget-webkit-last-session) 'vertical t -50))
>  
>  (defun xwidget-webkit-scroll-forward ()
> -  "Scroll webkit forwards.
> -Depending on the value of `xwidget-webkit-scroll-behavior',
> -this scrolls in `native' fashion, or like `image-mode' would."
> +  "Scroll webkit forwards."
>    (interactive)
> -  (if (eq xwidget-webkit-scroll-behavior 'native)
> -      (xwidget-set-adjustment (xwidget-webkit-last-session) 'horizontal t 50)
> -    (xwidget-webkit-scroll-forward)))   ; FIXME infloop!
> +  (xwidget-set-adjustment (xwidget-webkit-last-session) 'horizontal t 50))
>  
>  (defun xwidget-webkit-scroll-backward ()
> -  "Scroll webkit backwards.
> -Depending on the value of `xwidget-webkit-scroll-behavior',
> -this scrolls in `native' fashion, or like `image-mode' would."
> +  "Scroll webkit backwards."
>    (interactive)
> -  (if (eq xwidget-webkit-scroll-behavior 'native)
> -      (xwidget-set-adjustment (xwidget-webkit-last-session) 'horizontal t -50)
> -    (xwidget-webkit-scroll-backward))) ; FIXME infloop!
> +  (xwidget-set-adjustment (xwidget-webkit-last-session) 'horizontal t -50))
>  
>  
>  ;; The xwidget event needs to go into a higher level handler
> @@ -417,9 +394,7 @@ xwidget-webkit-adjust-size-to-content
>  (defun xwidget-webkit-adjust-size-dispatch ()
>    "Adjust size according to mode."
>    (interactive)
> -  (if (eq xwidget-webkit-scroll-behavior 'native)
> -      (xwidget-webkit-adjust-size-to-window)
> -    (xwidget-webkit-adjust-size-to-content))
> +  (xwidget-webkit-adjust-size-to-window)
>    ;; The recenter is intended to correct a visual glitch.
>    ;; It errors out if the buffer isn't visible, but then we don't get
>    ;; the glitch, so silence errors.

-- 
Joakim Verona

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.