Closed Bug 1321769 Opened 3 years ago Closed 2 years ago

stylo: scrollbars don't work


(Core :: CSS Parsing and Computation, defect, P1)






(Reporter: heycam, Assigned: TYLin)


(Blocks 1 open bug)



(1 file)

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?
Flags: needinfo?(cam)
No, scroll bars aren't visible at all now in Linux.
Flags: needinfo?(cam)
Priority: -- → P2
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.
Duplicate of this bug: 1341095
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.
[1] 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 [2] to make SliderFrame and ButtonBoxFrame generated in stylo.

With above, the scrollbar is visible [3] but it is still malfunction. It cannot be dragged and its position relative to the document is wrong. Perhaps some properties are in [4] are still not supported.

[3] On Mac, you need to change Settings -> General -> Show scroll bars -> Always. Some dynamic feature is missing in stylo...
> 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?
Depends on: 1321754
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.
Flags: needinfo?(tlin)
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.
Flags: needinfo?(tlin)
Blocks: 1351978
Duplicate of this bug: 1361045
Manish, did bug 1321754 fix the scrollbar issues you were seeing?
Flags: needinfo?(manishearth)
Ok, NI TY and Hiro to figure out what's going on here.
Flags: needinfo?(tlin)
Flags: needinfo?(hikezoe)
The difference is light gray bars inside scrollbars. I guess it's caused by lack of -moz-appearance support (bug 1349651).
Depends on: 1349651
Flags: needinfo?(hikezoe)
Manish, can you hack up a patch to change the line at [1] 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.

Flags: needinfo?(manishearth) is the same thing without the test expectations all marked as passing (so we can see the potential impact of this change)
Flags: needinfo?(manishearth)
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?
Flags: needinfo?(hikezoe)
Going to try a push which breaks both gecko and stylo the same way, to remove all effects of HasAuthorSpecifiedRules on the failures.
Here is a try with tweaks that -moz-appearance in vertical scrollbar styles is removed.

(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.
Flags: needinfo?(hikezoe)
FWIW, a list that properties in nsCSSProps::kMozAppearanceKTable but not in -moz-appearance keywords for stylo.

(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.
Depends on: 1361632
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.
Flags: needinfo?(tlin)
No longer blocks: 1351978
I don't see new issue related to scroll bar.
Closed: 2 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.