Closed Bug 1486662 Opened 3 years ago Closed 3 years ago

scrollbar-width: none affects scrollability on Android

Categories

(Core :: Widget: Android, defect)

Unspecified
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox63 --- wontfix
firefox64 --- fixed

People

(Reporter: xidorn, Assigned: xidorn)

References

Details

Attachments

(2 files)

It seems elements with scrollbar-width: none are not scrollable on Android for some reason.
Attached file test page
See this test page. All six areas should be scrollable on the two directions, however the two with scrollbar-width: none are not scrollable on Firefox Android.
James, could you provide some information about what to investigate for this issue?

"scrollbar-width: none" is designed to hide the scrollbars, but don't affect the scrollability of the element (unlike overflow: hidden). It currently works as expected on desktop platforms, but on Android, the element becomes not scrollable with it.

Currently, when scrollbar-width: none is specified on an element, we suppress the scrollbar via not generating the scrollbar elements[1]. It's not clear to me how scrollbars work on Android, and why suppressing them can lead to not scrollable.

Could you give some pointer about this?


[1] https://searchfox.org/mozilla-central/rev/5a18fb5aeeec99f1ca1c36a697082c221189a3b9/layout/generic/nsGfxScrollFrame.cpp#4678-4680
Flags: needinfo?(snorp)
I don't really have in-depth knowledge of how this works, but Botond and Kats should have an idea.
Flags: needinfo?(snorp)
Flags: needinfo?(kats)
Flags: needinfo?(botond)
https://searchfox.org/mozilla-central/rev/5a18fb5aeeec99f1ca1c36a697082c221189a3b9/layout/generic/nsGfxScrollFrame.cpp#1424

Most likely the scrollbar box elements are null here. The condition will need to be refined to distinguish between overflow:hidden (which is what it's trying to detect) and the scrollbar width being 0.
Flags: needinfo?(kats)
Oh actually the overflow check happens a few lines up. I don't recall offhand why we check for the boxes then.
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #1)
> Created attachment 9004424 [details]
> test page
> 
> See this test page. All six areas should be scrollable on the two
> directions, however the two with scrollbar-width: none are not scrollable on
> Firefox Android.

Note that reproducing this in a nightly build requires flipping layout.scrollbar-width.enabled to true.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) (parental leave) from comment #5)
> Oh actually the overflow check happens a few lines up. I don't recall
> offhand why we check for the boxes then.

Thankfully, the comment just above the linked line explains it :)

  // The check for scroll bars was added in bug 825692 to prevent layerization
  // of text inputs for performance reasons.

And just above we have:

  #if defined(MOZ_WIDGET_ANDROID)
    // Mobile platforms need focus to scroll.
    bool canScrollWithoutScrollbars = IsFocused(mOuter->GetContent());
  #else
    bool canScrollWithoutScrollbars = true;
  #endif

So basically, on Android, if a scrollable element doesn't have a scrollbar, it can only be scrolled if it's focused.

I guess, unlike a text input, a plain <div> cannot be focused, and so we can never scroll it if it doesn't have scrollbars.

Since this was written with text inputs in mind, it would be appropriate to relax this to always allow scrolling things that are not text inputs. That is, the first branch of the #if can be changed to:

    // Mobile platforms need focus to scroll text inputs.
    bool canScrollWithoutScrollbars = /* mOuter->GetContent() is not a text input */ ||
                                      IsFocused(mOuter->GetContent());

(with a suitable implementation of /* mOuter->GetContent() is not a text input */).
Flags: needinfo?(botond)
Thanks for the information!
Assignee: nobody → xidorn+moz
Attachment #9006785 - Attachment description: Bug 1486662 - Allow scroll without scrollbar for non-input elements on Android. r=botond → Bug 1486662 - Allow scroll without scrollbar for non-textcontrol elements on Android. r=tnikkel
Comment on attachment 9006785 [details]
Bug 1486662 - Allow scroll without scrollbar for non-textcontrol elements on Android. r=tnikkel

Timothy Nikkel (:tnikkel) has approved the revision.
Attachment #9006785 - Flags: review+
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a2d89aea7cce
Allow scroll without scrollbar for non-textcontrol elements on Android. r=tnikkel
https://hg.mozilla.org/mozilla-central/rev/a2d89aea7cce
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
I think this can ride the trains, but feel free to nominate for Beta approval if you feel strongly otherwise.
You need to log in before you can comment on or make changes to this bug.