752850 - [Responsive Mode] It should be possible to hide the scrollbars

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Paul Rouget [:paul]]

To reproduce a "mobile experience", we should let the user hide the scrollbars.

Comment 1

[Paul Rouget [:paul]]

I think it is now possible to just display:none the scrollbars. See bug 737003.

Comment 2

[Paul Rouget [:paul]]

Apparently, it doesn't work. I started a thread about that on dev-platform:
https://groups.google.com/d/topic/mozilla.dev.platform/wGMjWhHxKtE/discussion

Comment 3

[Paul Rouget [:paul]]

We found a solution:

let win = gBrowser.contentWindow;
let gIOService = Components.classes["@mozilla.org/network/io-service;1"].getService(Components.interfaces.nsIIOService);
let windowUtils = win.QueryInterface(Components.interfaces.nsIInterfaceRequestor).getInterface(Components.interfaces.nsIDOMWindowUtils);
let uri = gIOService.newURI('data:text/css,@namespace url("http://www.w3.org/1999/xhtml");@namespace xul url("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul");html xul|scrollbar {display:none}', null, null);
windowUtils.loadSheet(uri, windowUtils.AGENT_SHEET);
gBrowser.selectedBrowser.setAttribute("style", "display:none");
window.getComputedStyle(gBrowser.selectedBrowser).display;
gBrowser.selectedBrowser.removeAttribute("style");

Comment 4

[Paul Rouget [:paul]]

So now we have a working mechanism, we need to figure out what we could do for the user interface.

We could have a checkbox. But I don't like checkboxes.

We could hide the scrollbars automatically when we are below 600px. Does sound terrible.

Any idea?

Comment 5

[hsablonniere]

Nice work.

Why 600px? Automatically => :-(

What about some kind of switch? With :

Scrollbars : YES | NO

It's not really part of the default UX of Firefox though, isn't it?

BTW the same could be nice for orientation. With landscape and portrait.

Comment 6

[Paul Rouget [:paul]]

Attachment: Scratchpad

This scratchpad adds small scrollbars that overlaps content (like on mobile browser).

Styling scrollbars with CSS appears to be a little fragile. I didn't manage to get horizontal scrollbars. I also tried to get the rounded thumbs, but didn't succeed. But the current approach is satisfying.

Comment 7

[Paul Rouget [:paul]]

(In reply to hsablonniere from comment #5)
> Why 600px?

I don't know. Maybe we could just hide the scrollbar (or get the overlapping scrollbar) as soon as we enter the responsive mode.

> Automatically => :-(

Why not?

> What about some kind of switch? With :
> 
> Scrollbars : YES | NO
> 
> It's not really part of the default UX of Firefox though, isn't it?

Nope. But we can build one.

> BTW the same could be nice for orientation. With landscape and portrait.

Maybe.

So yeah… as you know, I'm not keen on adding UI elements.

I see 2 options:

1) we know that the scrollbar is not really useful in this specific case (testing responsive design), so let's just hide it as soon as we enter this mode.
2) we think it's better to have the choice, so then let's put a checkbox.

Comment 8

[Paul Rouget [:paul]]

(the current scratchpad is not working very well when the scrollbar is inside the content)

Comment 9

[Paul Rouget [:paul]]

Attachment: Simulate rounded overlapping scrollbars

Use a chrome scratchpad to test this (https://developer.mozilla.org/en/Tools/Scratchpad#Using_Scratchpad_to_access_Firefox_internals).

Comment 10

[Girish Sharma [:Optimizer]]

Attachment: ugly scrolbars

looks ugly on windows :(

Comment 11

[Paul Rouget [:paul]]

Attachment: v0.1

Comment 12

[Paul Rouget [:paul]]

Todo:
- windows & linux support
- support reload of the top level window and inner windows
- support dynamic creation of windows

Comment 13

[Paul Rouget [:paul]]

Comment on attachment 665996 [details] [diff] [review]
v0.1

Hubert, can you try this patch and tell me what you think?

Comment 14

[Girish Sharma [:Optimizer]]

This works on windows : 
http://pastebin.mozilla.org/1848168

Though I have not tested it on a rtl locale to properly see what -moz-box-align: start; could have impacted.

Comment 15

[Paul Rouget [:paul]]

Attachment: v1

Comment 16

[Paul Rouget [:paul]]

Optimizer, I didn't include your changes. Can I ask you to modify this patch to make it work on Windows? If you can just add a patch I can `hg qfold` into this one, that would help a lot :)

Let me know!

Comment 17

[Paul Rouget [:paul]]

https://tbpl.mozilla.org/?tree=Try&rev=6ae4092d0e98

Comment 18

[Paul Rouget [:paul]]

Comment on attachment 666904 [details] [diff] [review]
v1

Joe, this has been tested on mac only for now (we know it doesn't work well on windows). Some more patches are coming.

Comment 19

[Paul Rouget [:paul]]

Attachment: v1.1

Comment 20

[Dão Gottwald [:dao]]

Comment on attachment 666912 [details] [diff] [review]
v1.1

>--- a/browser/themes/winstripe/jar.mn
>+++ b/browser/themes/winstripe/jar.mn

>         skin/classic/aero/browser/devtools/tools-icons-small.png     (devtools/tools-icons-small.png)
>+        skin/classic/browser/devtools/floating-scrollbars.css        (devtools/floating-scrollbar.scss)

This is broken.

Comment 21

[Paul Rouget [:paul]]

Attachment: v1.2

Comment 22

[Joe Walker [:jwalker]]

Comment on attachment 666912 [details] [diff] [review]
v1.1

Review of attachment 666912 [details] [diff] [review]:
-----------------------------------------------------------------

Did you miss out browser/themes/gnomestripe/jar.mn

::: browser/devtools/shared/FloatingScrollbars.jsm
@@ +5,5 @@
> +"use strict";
> +
> +const { classes: Cc, interfaces: Ci, utils: Cu } = Components;
> +
> +const EXPORTED_SYMBOLS = [ "SwitchToFloatingScrollbars", "SwitchToNativeScrollbars" ];

I think I would have done

    Scrollbars = { useFloating: ..., useNative: ... };

I'm not sure that it's important, and I wouldn't r- because of it, but it does feel more contained to me.

I do think that if we're exporting functions then we should use lower initials so they don't get confused with constructors. So at the very least I think we should do

    const EXPORTED_SYMBOLS = [ "switchToFloatingScrollbars", "switchToNativeScrollbars" ];

Although 

    const EXPORTED_SYMBOLS = [ "Scrollbars" ];

feels better still.

@@ +40,5 @@
> +    mgr.reset();
> +  }
> +}
> +
> +function _ScrollbarManager(aTab) {

Since this isn't exported, do we need the initial _?

::: browser/themes/gnomestripe/devtools/floating-scrollbars.css
@@ +1,4 @@
> +@namespace url("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul");
> +
> +scrollbar {
> +  -moz-appearance: none !important;

I think we should document why we need to use !important. Or at the very least why some of these properties need it but not others.

Comment 23

[Paul Rouget [:paul]]

I prefer to avoid something like "scrollbar". I want the name of the exported symbol(s) to be very explicit as it only does one thing.

Ok for the lower case and the "_".

We don't document the "!important" anywhere. See for example https://mxr.mozilla.org/mozilla-central/source/mobile/xul/themes/core/content.css or http:mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/content.css

Comment 24

[Paul Rouget [:paul]]

Attachment: v1.3

Comment 25

[Paul Rouget [:paul]]

Attachment: v1.4

Comment 26

[Paul Rouget [:paul]]

(typo in jar.man)

Comment 27

[Joe Walker [:jwalker]]

(In reply to Paul Rouget [:paul] from comment #23)
> We don't document the "!important" anywhere. See for example
> https://mxr.mozilla.org/mozilla-central/source/mobile/xul/themes/core/
> content.css or
> http:mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/content.css

I'm still left wondering why we needed !important, and why we needed it some places and not others.
We could have that discussion here, but I'm guessing that I might not be the only person to benefit from the thinking.

Comment 28

[Paul Rouget [:paul]]

(In reply to Joe Walker [:jwalker] from comment #27)
> I'm still left wondering why we needed !important, and why we needed it some
> places and not others.
> We could have that discussion here, but I'm guessing that I might not be the
> only person to benefit from the thinking.

I just copied the code we use for mobile. I will remove all the !important and see which ones are really useful.

Comment 29

[Paul Rouget [:paul]]

Attachment: v1.5

removed all the important. Only useful for thumb. But I don't know why.

Comment 30

[Girish Sharma [:Optimizer]]

Attachment: Patch v1.5 with Windows fixes

Same as patch v1.5 for other OS than Windows. Made all the changes required for windows. Removed !importants where not required.

Comment 31

[Paul Rouget [:paul]]

(I'm ok with Optmizer's changes)

Comment 32

[Paul Rouget [:paul]]

Comment on attachment 666932 [details] [diff] [review]
Patch v1.5 with Windows fixes

Cancelling the review until I get my new laptop to test on Linux and Windows (apparently, this patch doesn't work at all on Linux).

Comment 33

[Paul Rouget [:paul]]

Attachment: v1.6

Comment 34

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: v1.7 - Fixed Linux issues

Comment 35

[Paul Rouget [:paul]]

Comment on attachment 666945 [details] [diff] [review]
v1.7 - Fixed Linux issues

Thank you guys!

Comment 36

[Panos Astithas (he/him) [:past] (please ni?)]

https://hg.mozilla.org/integration/fx-team/rev/1332149e38af

Comment 37

[Panos Astithas (he/him) [:past] (please ni?)]

https://hg.mozilla.org/mozilla-central/rev/1332149e38af

Comment 38

[Chris Lord [:cwiiis]]

Should this be reopened? Currently scrollbars are shown and there's no way to turn them off.

Comment 39

[Paul Rouget [:paul]]

(In reply to Chris Lord [:cwiiis] from comment #38)
> Should this be reopened? Currently scrollbars are shown and there's no way
> to turn them off.

See bug 799471.

Comment 40

[Alex Shilov]

Excuse me, Is there possible way to enable this implementation into Firefox default view? In fact, this is exactly the scrollbar style that I want on my websites, and I'm doing something similar to this for Chrome using ::-webkit-scrollbar rules, because this looks way better than ugly windows classic gray scrollbars.
This could be possible a fix for bug 77790, bug 547260, bug 384458, bug 21890, bug 259640, and even bug 590945 can be related somehow.