1486662 - scrollbar-width: none affects scrollability on Android

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Android, defect)

Description

[Xidorn Quan [:xidorn] UTC+10]

It seems elements with scrollbar-width: none are not scrollable on Android for some reason.

Comment 1

[Xidorn Quan [:xidorn] UTC+10]

Attachment: 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.

Comment 2

[Xidorn Quan [:xidorn] UTC+10]

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

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

I don't really have in-depth knowledge of how this works, but Botond and Kats should have an idea.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Oh actually the overflow check happens a few lines up. I don't recall offhand why we check for the boxes then.

Comment 6

[Botond Ballo [:botond]]

(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.

Comment 7

[Botond Ballo [:botond]]

(In reply to Kartikaya Gupta (email:[e-mail]) (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 */).

Comment 8

[Xidorn Quan [:xidorn] UTC+10]

Thanks for the information!

Comment 9

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1486662 - Allow scroll without scrollbar for non-textcontrol elements on Android. r=tnikkel

Comment 10

[Timothy Nikkel (: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.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a2d89aea7cce
Allow scroll without scrollbar for non-textcontrol elements on Android. r=tnikkel

Comment 12

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/a2d89aea7cce

Comment 13

[Ryan VanderMeulen [:RyanVM]]

I think this can ride the trains, but feel free to nominate for Beta approval if you feel strongly otherwise.