Allow autoscrolling on XUL pages (about:addons, about:preferences, etc)

RESOLVED FIXED in Firefox 59

Status

()

Toolkit
XUL Widgets
P3
normal
RESOLVED FIXED
10 months ago
7 months ago

People

(Reporter: botond, Assigned: arai)

Tracking

unspecified
mozilla59
Points:
---

Firefox Tracking Flags

(firefox55 wontfix, firefox56 wontfix, firefox57 wontfix, firefox58 wontfix, firefox59 fixed)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

10 months ago
See bug 1386742 comments 18 through 20.

The "Extensions" section of about:addons is an example of something that can't be autoscrolled today, but we would like it to be.

Updated

9 months ago
status-firefox55: --- → affected
status-firefox56: --- → affected
status-firefox57: --- → affected
Priority: -- → P3

Comment 1

8 months ago
Sorry if this is a "me too", but the recent change to the Options/Preferences screen, where there are fewer categories but each one is now much longer, is another screen that could benefit from autoscrolling.
Duplicate of this bug: 1416264

Updated

7 months ago
status-firefox58: --- → wontfix
status-firefox59: --- → fix-optional
(Assignee)

Comment 3

7 months ago
modifying summary for searchability.
Summary: Allow autoscrolling on XUL pages → Allow autoscrolling on XUL pages (about:addons, about:preferences, etc)
(Assignee)

Updated

7 months ago
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
(Assignee)

Comment 4

7 months ago
Created attachment 8933902 [details] [diff] [review]
Part 1: Allow autoscrolling on XUL pages (about:addons, about:preferences, etc)

This allows scrolling in most places, but this skips XBL elements, so scrollbox in XBL is ignored,
that results in unable to scroll in about:addons richlistbox, starting from richlistitem.
(Assignee)

Comment 5

7 months ago
tree is also not scrollable even after supporting XBL... and autoscroll in about:config doesn't work :/
(Assignee)

Comment 6

7 months ago
Created attachment 8933908 [details] [diff] [review]
Part 1: Allow autoscrolling on XUL elements.

This patch allows XUL elements, except elements in XBL.
Attachment #8933902 - Attachment is obsolete: true
Attachment #8933908 - Flags: review?(jaws)
(Assignee)

Comment 7

7 months ago
Created attachment 8933909 [details] [diff] [review]
Part 2: Allow autoscrolling on XBL elements.

This patch supports traversing XBL elements, so that richlistbox can be scrolled.
Attachment #8933909 - Flags: review?(jaws)
(Assignee)

Comment 8

7 months ago
tree uses standalone scrollbar, so I think it needs special support, that I think should be done in separate bug.
(Assignee)

Comment 9

7 months ago
Comment on attachment 8933909 [details] [diff] [review]
Part 2: Allow autoscrolling on XBL elements.

Review of attachment 8933909 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/content/browser-content.js
@@ +105,5 @@
> +    }
> +    return false;
> +  },
> +
> +  *parentNodeIterator(aNode) {

too bad that generator-star-spacing rule rejects this :/
I've put a space locally.
(Assignee)

Comment 10

7 months ago
Comment on attachment 8933909 [details] [diff] [review]
Part 2: Allow autoscrolling on XBL elements.

sorry, this seems to be broken.
I'll update shortly
Attachment #8933909 - Attachment is obsolete: true
Attachment #8933909 - Flags: review?(jaws)
(Assignee)

Comment 11

7 months ago
Created attachment 8934038 [details] [diff] [review]
Part 2: Allow autoscrolling on XBL elements.

This patch supports traversing XBL elements, so that richlistbox can be scrolled.

(from previous patch)
fixed when to update this._scrollable.
this._scrollable is now set to null first, for the case when there's no scrollable node.
and set to current node in iteration when the node is scrollable.
Attachment #8934038 - Flags: review?(jaws)
Comment on attachment 8933908 [details] [diff] [review]
Part 1: Allow autoscrolling on XUL elements.

Review of attachment 8933908 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/content/browser-content.js
@@ +78,5 @@
> +  isScrollableElement(aNode) {
> +    if (aNode instanceof content.HTMLElement) {
> +      if (aNode instanceof content.HTMLSelectElement && !aNode.multiple) {
> +        return false;
> +      }

This can be simplified to just:
`return !(aNode instanceof content.HTMLSelectElement) || aNode.multiple;`

@@ +84,5 @@
> +      return true;
> +    }
> +
> +    if (aNode instanceof content.XULElement) {
> +      return true;

This can be simplified to just:
`return aNode instanceof content.XULElement;`
Attachment #8933908 - Flags: review?(jaws) → review+
Comment on attachment 8934038 [details] [diff] [review]
Part 2: Allow autoscrolling on XBL elements.

Review of attachment 8934038 [details] [diff] [review]:
-----------------------------------------------------------------

Looks like it should work and it's early enough in the Nightly cycle that I'm fine with taking this.

::: toolkit/content/browser-content.js
@@ +139,2 @@
>        // do not use overflow based autoscroll for <html> and <body>
>        // Elements or non-html elements such as svg or Document nodes

This comment needs updating (the non-HTML part).
Attachment #8934038 - Flags: review?(jaws) → review+
Backed out for devtools mochitest failure devtools/client/debugger/test/mochitest/browser_dbg_search-symbols.js  r=backout a=backout on a CLOSED TREE

Failure push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=28f34951d1b3fc6c0399435f355dd9ae5a12b3fb
Failure log: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=28f34951d1b3fc6c0399435f355dd9ae5a12b3fb

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/a26e71300a88
Flags: needinfo?(arai.unmht)
Duplicate of this bug: 1423179

Comment 17

7 months ago
When will it go into Nightly, Tooru?

Comment 18

7 months ago
Pushed by arai_a@mac.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a00f8ced0151
Part 1: Allow autoscrolling on XUL elements. r=jaws
https://hg.mozilla.org/integration/mozilla-inbound/rev/286197fbaa34
Part 2: Allow autoscrolling on XBL elements. r=jaws
(Assignee)

Comment 20

7 months ago
(In reply to Andreea Pavel [:apavel] from comment #15)
> Backed out for devtools mochitest failure
> devtools/client/debugger/test/mochitest/browser_dbg_search-symbols.js 
> r=backout a=backout on a CLOSED TREE

it's somehow from bug 1228841.
Flags: needinfo?(arai.unmht)

Comment 21

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a00f8ced0151
https://hg.mozilla.org/mozilla-central/rev/286197fbaa34
Status: ASSIGNED → RESOLVED
Last Resolved: 7 months ago
status-firefox59: fix-optional → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59

Comment 22

7 months ago
When will it come to Firefox Nightly, Dorel?
(In reply to Daniel from comment #22)
> When will it come to Firefox Nightly, Dorel?

The change can be found in today's Nightly build (2017-12-07).

Comment 24

7 months ago
Sorry, I meant developer edition, not nightly.
developer edition should get this change when it moves to 59 by mid january
status-firefox55: affected → wontfix
status-firefox56: affected → wontfix
status-firefox57: affected → wontfix

Comment 26

7 months ago
Oh wooow so long till then. Okay thank you.
You need to log in before you can comment on or make changes to this bug.