Closed Bug 1321769 Opened 3 years ago Closed 2 years ago
stylo: scrollbars don't work
On Linux at least, scrollbars don't work in Servo-styled documents. They appear, but the thumb doesn't move or get resized appropriately.
Was this fixed by bug 132128, or is there still more to do here?
No, scroll bars aren't visible at all now in Linux.
A testcase from bz in bug 1341095: data:text/html,<div style="width: 200%; height: 20px; background: blue"></div> It sounds like this is not restricted to linux.
Ting-Yu, can you take a look at this one?
Assignee: nobody → tlin
Priority: P2 → P1
Ah, bug 1321754 comment 0 indicates that that bug is the cause of this one. Assigning to Ting-yu.
Here are the list of reftest failures caused by this.
 is the stylo vs gecko frame tree dump for the test case in comment 3. In stylo, no SliderFrame and ButtonBoxFrame is generated under ScrollbarFrame. We can simply change the SheetParsingMode of scrollbars.css from eAuthorSheetFeatures to eAgentSheetFeatures in  to make SliderFrame and ButtonBoxFrame generated in stylo. With above, the scrollbar is visible  but it is still malfunction. It cannot be dragged and its position relative to the document is wrong. Perhaps some properties are in  are still not supported.  https://pastebin.mozilla.org/8983289  http://searchfox.org/mozilla-central/rev/7419b368156a6efa24777b21b0e5706be89a9c2f/layout/style/nsLayoutStylesheetCache.cpp#78-79  On Mac, you need to change Settings -> General -> Show scroll bars -> Always. Some dynamic feature is missing in stylo...  http://searchfox.org/mozilla-central/source/toolkit/themes/osx/global/nativescrollbars.css
> We can simply change the SheetParsingMode of scrollbars.css from eAuthorSheetFeatures to eAgentSheetFeatures We can, but how about we fix bug 1321754 instead of increasing our security exposure?
How much work is there to do after bug 1321754 to get scrollbars working? If it's significant, it might make sense to parallelize it given that so many tests are failing due to scrollbar issues. Jeremy has some cycles to work on high-priority stuff, so let us know if you think it would make sense to have him work on some of this.
Recently, after applying my patch in bug 1321754, the scrollbar is working at least on Linux and Mac. The issues I described in comment 8 no longer exist. Dragging the scrollbar and the position is working now. I don't think there's much to do after bug 1321754 lands, but I'll push a try to see whether there's anything significant left.
Manish, did bug 1321754 fix the scrollbar issues you were seeing?
Nope. https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=https://queue.taskcluster.net/v1/task/csLA_yz8TuaE4eAlKphjyw/runs/0/artifacts/public/logs/live_backing.log&only_show_unexpected=1 (See first few tests, e.g. layout/reftests/reftest-sanity/656041-1.html) layout/reftests/invalidation/transform-floating-point-invalidation.html in https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=https://queue.taskcluster.net/v1/task/TIkFStnWTYisYAyOFRJ-Ow/runs/0/artifacts/public/logs/live_backing.log&only_show_unexpected=1 The scrollbar failures still seem to exist everywhere.
Ok, NI TY and Hiro to figure out what's going on here.
The difference is light gray bars inside scrollbars. I guess it's caused by lack of -moz-appearance support (bug 1349651).
Depends on: 1349651
Manish, can you hack up a patch to change the line at  to |return false;| and re-do your try push from comment 14? It would be good to know if most/all of those scrollbar failures will be fixed by HASR, or whether there are other things we should be working on in parallel.  http://searchfox.org/mozilla-central/rev/abe68d5dad139e376d5521ca1d4b7892e1e7f1ba/layout/style/nsRuleNode.cpp#10500
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8d60c51ba9bb3c9f6e020973ea9bcdba82eca100 https://treeherder.mozilla.org/#/jobs?repo=try&revision=ac8e58ffc537665afe44714ac751dd7d77ffc547 is the same thing without the test expectations all marked as passing (so we can see the potential impact of this change)
Nope. layout/reftests/reftest-sanity/656041-1.html still fails there, and most of the failures are still scrollbar failures. Hiro, is there something else we may have missed?
Going to try a push which breaks both gecko and stylo the same way, to remove all effects of HasAuthorSpecifiedRules on the failures. https://treeherder.mozilla.org/#/jobs?repo=try&revision=e7c5d9e5716c66f5e16021027151c34e1e16faf8
Here is a try with tweaks that -moz-appearance in vertical scrollbar styles is removed. https://treeherder.mozilla.org/#/jobs?repo=try&revision=3ebc8e8cde1eac643f6fae030d71d5e1feddcb62 (layout/reftests/reftest-sanity/656041-1.html passed there) So, I still believe -moz-appearance is needed for those reftests. Oh, I just realized that scrollbar-horizontal and scrollbar-vertical are not listed in supported -moz-appearance keywords. https://hg.mozilla.org/mozilla-central/file/a748acbebbde/servo/components/style/properties/longhand/box.mako.rs#l2404
FWIW, a list that properties in nsCSSProps::kMozAppearanceKTable but not in -moz-appearance keywords for stylo. -moz-gtk-info-bar -moz-mac-active-source-list-selection -moz-mac-disclosure-button-closed -moz-mac-disclosure-button-open -moz-mac-fullscreen-button -moz-mac-help-button -moz-mac-source-list -moz-mac-source-list-selection -moz-mac-vibrancy-dark -moz-mac-vibrancy-light dialog scrollbar scrollbar-horizontal scrollbar-small scrollbar-vertical window
(In reply to Hiroyuki Ikezoe (:hiro) from comment #22) > FWIW, a list that properties in nsCSSProps::kMozAppearanceKTable but not in > -moz-appearance keywords for stylo. Filed bug 1361632 for this.
After bug 1361632 was landed, scrollbar works pretty well on my Mac, which does the dynamic hide and show. Let's keep bug open, and see what else do we need to fix after HasAuthorSpecifiedRules is implemented.
I don't see new issue related to scroll bar.
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.