Remove the "scrollbox" anonymous element from "richlistbox"

RESOLVED FIXED in Firefox 66

Status

()

task
P3
normal
RESOLVED FIXED
Last year
20 days ago

People

(Reporter: Paolo, Assigned: Paolo)

Tracking

(Blocks 1 bug)

Trunk
mozilla66
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox63 wontfix, firefox64 wontfix, firefox65 wontfix, firefox66 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Assignee

Description

Last year
The "scrollbox" element in "richlistbox" is a legacy artifact and probably not needed anymore, since its scrolling features are now available on all normal CSS frames.

This is the only anonymous element in "richlistbox", and removing this will unblock the conversion to Custom Element.
Assignee

Updated

Last year
Blocks: 1472558
Assignee

Comment 1

9 months ago
Posted patch Work in progress (obsolete) — Splinter Review
Does this generally work with the patch applied? This would be great to do since I'm guessing Bug 1472558 would be relatively easy after this, now that we can implement the interface on it.
Flags: needinfo?(paolo.mozmail)
Assignee

Comment 3

8 months ago
No, there are two things to figure out:

1. Since "position: sticky;" is not supported in XUL layout (bug 987339), we need a way to keep the headers fixed in the few places where we actually have them. If there is no easy way to do that without anonymous content, we can still solve this by moving the headers outside of the <richlistbox> element. We need to touch the consumers anyways at some point, since we have to migrate them away from <treecol>. Example:

https://searchfox.org/mozilla-central/rev/c9272ef398954288525e37196eada1e5a93d93bf/browser/components/preferences/permissions.xul#56-60

2. We need to replace the scrollBy and scrollToElement calls with something else. This is probably quite easy to reimplement using standard DOM methods.
Flags: needinfo?(paolo.mozmail)
Assignee

Updated

6 months ago
Depends on: 1516258
Assignee

Updated

6 months ago
Depends on: 1516442
Assignee

Updated

6 months ago
Attachment #9011792 - Attachment is obsolete: true
Assignee

Updated

6 months ago
Depends on: 1516763
Once this lands, the CSS changes in https://phabricator.services.mozilla.com/D15388 will need to be ported to TB - mostly this is flattening `richlistbox > scrollbox` selectors to `richlistbox` (see https://searchfox.org/comm-central/search?q=%3E+scrollbox&path=).

In some cases margins that were previously split between the two elements will need to be merged into just `richlistbox` (see toolkit/themes/linux/mozapps/update/updates.css).
Flags: needinfo?(jorgk)

Updated

6 months ago
Depends on: 1516813

Comment 8

6 months ago
Thanks for the heads-up, I've filed bug 1516813.
Flags: needinfo?(jorgk)
Assignee

Comment 9

6 months ago
I have a XUL reftest failure on Android only, I'll disable just this test for now:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8ebbf40ddbec11d28f79ac681c1eeac0071d7c1b&selectedJob=219293558

Brian, what is the latest thinking in bug 1465457 about disabling more XUL reftests? It's not clear to me how much of XUL is still used on Android, I just see a minimal "browser.xul" file there, and maybe other regression tests already cover this usage without any need for reftests.
Flags: needinfo?(bgrinstead)

Comment 10

6 months ago
Pushed by paolo.mozmail@amadzone.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8b2453d488ae
Remove the "scrollbox" anonymous element from "richlistbox". r=bgrins

Comment 11

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8b2453d488ae
Status: NEW → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Assignee: nobody → paolo.mozmail

(In reply to :Paolo Amadini from comment #9)

I have a XUL reftest failure on Android only, I'll disable just this test
for now:

https://treeherder.mozilla.org/#/
jobs?repo=try&revision=8ebbf40ddbec11d28f79ac681c1eeac0071d7c1b&selectedJob=2
19293558

Brian, what is the latest thinking in bug 1465457 about disabling more XUL
reftests? It's not clear to me how much of XUL is still used on Android, I
just see a minimal "browser.xul" file there, and maybe other regression
tests already cover this usage without any need for reftests.

Sorry for the delay. From everything I can gather, there's not really any reason for stuff that's XUL-specific to run on Android. The current patch in bug 1465457 skips anything that we flagged as "chrome" (i.e. run in the parent process over a chrome:// URI) in Android.

Flags: needinfo?(bgrinstead)

Updated

5 months ago
Depends on: 1521309
Depends on: 1524210
Depends on: 1537735
No longer depends on: 1537735
Regressions: 1537735

Updated

20 days ago
Type: enhancement → task
You need to log in before you can comment on or make changes to this bug.