1276629 - <select> dropdown buttons are not displayed if RDM is enabled

Status: VERIFIED FIXED

(DevTools :: Responsive Design Mode, defect, P1)

Description

[Mihai Boldan, Desktop QA [:mboldan]]

Attachment: dropdown.gif

[Note]:
- Logged based on Bug 1240913 Comment 56

[Affected versions]:
- Firefox 49.0a1 (2016-05-29)

[Affected platforms]:
- Windows 10 x64, Ubunut 16.04 x64 and Mac OS X 10.10.5

[Steps to reproduce]:
1. Launch Firefox.
2. From about:config, enable the devtools.responsive.html.enabled pref.
3. Go to http://www.ebay.com/ and observe the dropdown button displayed next to 'All Categories'.
4. Enable RDM. 

[Expected result]:
- The dropdown button is correctly displayed.

[Actual result]:
- The dropdown button is not displayed (See the attached video). 

[Regression range]:
- I'll investigate asap if this is a regression.

[Additional notes]:
- Here is another example - http://bikeshop.ro/src.php?lng=ro&tip=am

Comment 1

[Helen V. Holmes (:helenvholmes) (:✨)]

Attachment: modal.patch

Like I said in standup, this doesn't cover all of the items listed in the bug—I wasn't sure how to handle the overlay on top of the iframe (sure it's possible, just wasn't sure how to myself). (Thus, pointer events are still on.)

Comment 2

[Helen V. Holmes (:helenvholmes) (:✨)]

Sorry, wrong bug!

Comment 3

[Mihai Boldan, Desktop QA [:mboldan]]

I'm attaching the results from mozregression:
Last good revision: 68c0b7d6f16ce5bb023e08050102b5f2fe4aacd8
First bad revision: 8fc46323cee61bddc4c38701d6020379bba5774a
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=68c0b7d6f16ce5bb023e08050102b5f2fe4aacd8&tochange=8fc46323cee61bddc4c38701d6020379bba5774a

Probably Bug 1240896 - Switch to iframe mozbrowser caused this issue.

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

This issue is fixed by the work in bug 1240900.  Won't fix for earlier branches since it's only in new RDM.

Comment 5

[Camelia Badau [:cbadau], Desktop QA]

I've reproduced the initial issue on Nightly 49.0a1 (2016-05-29). 

I've tested on Windows 7 64bit, Ubuntu 14.04 64bit and Mac OSX 10.10.5 using latest Nightly 50.0a1 (buildID: 20160629030209) and the issue is still reproducible on both sites from Description: the dropdown button is not displayed if RDM is enabled.

Reopening the issue based on this.

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Camelia Badau, QA [:cbadau] from comment #5)
> I've reproduced the initial issue on Nightly 49.0a1 (2016-05-29). 
> 
> I've tested on Windows 7 64bit, Ubuntu 14.04 64bit and Mac OSX 10.10.5 using
> latest Nightly 50.0a1 (buildID: 20160629030209) and the issue is still
> reproducible on both sites from Description: the dropdown button is not
> displayed if RDM is enabled.
> 
> Reopening the issue based on this.

In my testing with the 2016-07-05 Nightly build, it looks like the case is:

* For macOS, the dropdown button does appear (works correctly)
* For Windows and Linux, the dropdown button is missing (broken like comment 0)

Is that the behavior you are seeing as well?

Comment 7

[Camelia Badau [:cbadau], Desktop QA]

I've tested on latest Nightly 50.0a1 (2016-07-05) on both sites from Description and I have the following results: 

- on Windows 7 64bit and Ubuntu 14.04 64bit - the dropdown button is missing (if RDM is enabled)
- on Mac OSX 10.9.5, Mac OSX 10.10.5, Mac OSX 10.11 - the dropdown button is missing (if RDM is enabled)
- on Mac OSX 10.12(Beta) - the dropdown button appears (if RDM is enabled)

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Bug in disabled feature under development, removing regression keyword.

Comment 9

[arni2033]

Underlying issue: bug 1222674
I still think it's possible to make some workaround on devtools side.

Comment 10

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Added another test case page to avoid relaying on eBay.

Comment 11

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

My fix here should also resolve bug 1222674.

Comment 12

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Bug 1276629 - Exclude select from floating scrollbars CSS.

Review commit: https://reviewboard.mozilla.org/r/85680/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/85680/

Comment 13

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c127abe6d908f9a98091bd2411fe6fb146522005

Comment 14

[Tim Nguyen :ntim]

Comment on attachment 8800861 [details]
Bug 1276629 - Exclude select from floating scrollbars CSS.

https://reviewboard.mozilla.org/r/85680/#review84720

Fixes the issue on my Windows 10 VM, thanks!

::: devtools/client/themes/floating-scrollbars-dark-theme.css:5
(Diff revision 1)
> +   scrollbars inside a <select> are excluded (including them distorts their
> +   appearance.  We want to include both the root scrollbars for the document as

Maybe "including them hides the select arrow on Windows" would be more precise than "including them distorts their appearance"

::: devtools/client/themes/floating-scrollbars-dark-theme.css:43
(Diff revision 1)
> -:root[platform="mac"] xul|slider {
> +:root[platform="mac"] html|*:not(html|select) > scrollbar slider {
>    -moz-appearance: none !important;
>  }
>  
> -:root[platform="win"] xul|scrollbar xul|scrollbarbutton,
> -:root[platform="linux"] xul|scrollbar xul|scrollbarbutton,
> -:root[platform="win"] xul|scrollbar xul|gripper,
> -:root[platform="linux"] xul|scrollbar xul|gripper {
> +:root[platform="win"] html|*:not(html|select) > scrollbar scrollbarbutton,
> +:root[platform="linux"] html|*:not(html|select) > scrollbar scrollbarbutton,
> +:root[platform="win"] html|*:not(html|select) > scrollbar gripper,
> +:root[platform="linux"] html|*:not(html|select) > scrollbar gripper {

I'm not even sure how the :root[platform="X"] even applies on HTML documents (eg. the inspector or the memory tool). Since the default namespace is XUL, this should only apply on XUL root elements.

But for some reason, the scrollbarbuttons and the gripper are still properly hidden on Windows (I haven't tested with Linux). Maybe another rule already hides the scroll-buttons and the gripper (eg. some kind of -moz-appearance: none rule?) and this rule is not applying ?

::: devtools/client/themes/floating-scrollbars-responsive-design.css:6
(Diff revision 1)
>  @namespace url("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul");
> +@namespace html url("http://www.w3.org/1999/xhtml");
>  
> -scrollbar {
> +/* Restrict all styles to `html|*:not(html|select) > scrollbar` so that
> +   scrollbars inside a <select> are excluded (including them distorts their
> +   appearance.  We want to include both the root scrollbars for the document as

Same comment about the wording

::: devtools/client/themes/floating-scrollbars-responsive-design.css:40
(Diff revision 1)
>    border-radius: 3px;
>    border-width: 0px !important;
>    border-radius: 3px !important;

Looks like there is some duplicate border-radius: 3px defined here, can you remove one of them while you're changing this?

Comment 15

[Tim Nguyen :ntim]

(In reply to Tim Nguyen :ntim (use needinfo?) from comment #14)
> ::: devtools/client/themes/floating-scrollbars-dark-theme.css:43
> (Diff revision 1)
> > -:root[platform="mac"] xul|slider {
> > +:root[platform="mac"] html|*:not(html|select) > scrollbar slider {
> >    -moz-appearance: none !important;
> >  }
> >  
> > -:root[platform="win"] xul|scrollbar xul|scrollbarbutton,
> > -:root[platform="linux"] xul|scrollbar xul|scrollbarbutton,
> > -:root[platform="win"] xul|scrollbar xul|gripper,
> > -:root[platform="linux"] xul|scrollbar xul|gripper {
> > +:root[platform="win"] html|*:not(html|select) > scrollbar scrollbarbutton,
> > +:root[platform="linux"] html|*:not(html|select) > scrollbar scrollbarbutton,
> > +:root[platform="win"] html|*:not(html|select) > scrollbar gripper,
> > +:root[platform="linux"] html|*:not(html|select) > scrollbar gripper {
> 
> I'm not even sure how the :root[platform="X"] even applies on HTML documents
> (eg. the inspector or the memory tool). Since the default namespace is XUL,
> this should only apply on XUL root elements.
*|*:root[platform="X"] would be the correct way to apply it to all namespaces I think.

> But for some reason, the scrollbarbuttons and the gripper are still properly
> hidden on Windows (I haven't tested with Linux). Maybe another rule already
> hides the scroll-buttons and the gripper (eg. some kind of -moz-appearance:
> none rule?) and this rule is not applying ?
To be clear, I've tested in the inspector which is an HTML doc.

Comment 16

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8800861 [details]
Bug 1276629 - Exclude select from floating scrollbars CSS.

https://reviewboard.mozilla.org/r/85680/#review84720

> Maybe "including them hides the select arrow on Windows" would be more precise than "including them distorts their appearance"

Makes sense, changed.

> I'm not even sure how the :root[platform="X"] even applies on HTML documents (eg. the inspector or the memory tool). Since the default namespace is XUL, this should only apply on XUL root elements.
> 
> But for some reason, the scrollbarbuttons and the gripper are still properly hidden on Windows (I haven't tested with Linux). Maybe another rule already hides the scroll-buttons and the gripper (eg. some kind of -moz-appearance: none rule?) and this rule is not applying ?

Hmm, it is a bit confusing to be sure whether `*|*:root` is needed here...  I added it for good measure, seems to be fine.

Looking at this again though, I guess we actually want to also match scrollbars in XUL elements if any, so I think I'll change to `*|*:not(html|select)` as well.

> Looks like there is some duplicate border-radius: 3px defined here, can you remove one of them while you're changing this?

Fixed, kept the !important one to match the other file.

Comment 17

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8800861 [details]
Bug 1276629 - Exclude select from floating scrollbars CSS.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/85680/diff/1-2/

Comment 18

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Build only run for checking results on Windows:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=07eb709893ac6968abe6efc70a0f2e75da79e7d9

Comment 19

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Updated version continues to work in my Windows 10 VM.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/17e075f4cd53
Exclude select from floating scrollbars CSS. r=ntim

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/17e075f4cd53

Comment 22

[Sayed [:LiquidO2]]

I have reproduced this bug with Nightly 49.0a1 (2016-05-30) in Elementary OS 64bit.

This bug's is now verified in Latest Nightly 52.0a1

Build ID 	20161020030211
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0

[testday-20161021]

Comment 23

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 49.0a1 (2016-05-30) in Windows 7 , 64 Bit !

This bug's is now verified in Latest Nightly 52.0a1

Build ID    :  20161025030205
User Agent  :  Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:52.0) Gecko/20100101 Firefox/52.0

[bugday-20161026]