874984 - Scrollbars repaint incorrectly on b2g desktop

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: General, defect)

Description

[Alexandre Poirot [:ochameau]]

Scrollbars are repainted inccorrectly on b2g desktop.
They are flickering when scrolling and looks broken.

Some reports have been made when using the Simulator:
  https://github.com/mozilla/r2d2b2g/issues/495

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Prevent content css to apply on chrome xul nodes and fix scrollbar appearance on windows.

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 752848 [details] [diff] [review]
Prevent content css to apply on chrome xul nodes and fix scrollbar appearance on windows.

I already r+ it :)

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

https://hg.mozilla.org/projects/birch/rev/780d65e3ebfa

Comment 4

[Alexandre Poirot [:ochameau]]

This patch introduce a reftest failure:
https://tbpl.mozilla.org/php/getParsedLog.php?id=23323602&tree=Birch

That's because we are overriding this test css:
http://mxr.mozilla.org/mozilla-central/source/layout/reftests/svg/foreignObject-form-theme.svg?raw=1

I do not know exactly how to address this.
We should either back this out or disable this test on b2g.

Comment 5

[Phil Ringnalda (:philor)]

After careful examination of the evidence for people's concern with b2g's SVG rendering in http://mxr.mozilla.org/mozilla-central/source/layout/reftests/svg/reftest.list, I marked it as fails-if(B2G) in https://hg.mozilla.org/projects/birch/rev/c0213576a3b7

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/780d65e3ebfa
https://hg.mozilla.org/mozilla-central/rev/c0213576a3b7

Comment 7

[Boris Zbarsky [:bzbarsky]]

This is adding a whole bunch of slow selectors to all web page rendering.  Why is that a good idea?

If you have rules that should apply only to some documents but not others, the right fix is to split them into separate sheets and only load the ones you care about.  Or to use @-moz-document rules so the determination is done only once at rule cascade creation time.  The right fix is NOT to add a bunch of runtime checking to every selector that will slow down pageload and the like!

Comment 8

[Alexandre Poirot [:ochameau]]

You are totally right, thanks for opening our eyes!

The story of this CSS is that it disables native look'n feel by setting moz-appearance to none and setting a custom nice looking, device friendly style to forms elements and scrollbars.
But on b2g desktop, it also applies to chrome XUL documents, whereas we want to keep native design for forms and scrollbars in this case.
It is applied on all documents as this CSS is registered as a global agent stylesheet:
  category agent-style-sheets browser-content-stylesheet chrome://browser/content/content.css
Ideally, we would like to apply it only on HTML documents, that's what we tried to achieve here by adding `html` at beginning of each rule.

Comment 9

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Basically the stylesheet should be loaded only for content that is not browser chrome related and should be able to affect anonymous nodes. 

Is there a way to do that? 

If not, does adding 'content-style-sheet' type at http://mxr.mozilla.org/mozilla-central/source/layout/base/nsStyleSheetService.cpp#221 that would filter on nsContentUtils::IsChromeDoc is a reasonable solution?

Comment 10

[Boris Zbarsky [:bzbarsky]]

(In reply to Alexandre Poirot (:ochameau) from comment #8)

Trying to style "html documents" differently is somewhat nonsensical.  For example, an SVG document that has a foreignObject containing a <select> or whatnot will have not <html> but will be web content all the same, and the patch will cause the rendering there to look different from other <select>s.

(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #9)
>Is there a way to do that? 

Most simply by having a userContent.css, but I assume that this is not desirable in this case because we want the sheet to be UA-level.

If you only wanted to sheet to apply to chrome you might have been able to do that with a moz-document rule using url-prefix(chrome://).

But there is no good infrastructure right now for styling only "content" (whatever that means in a world in which everything is actually "content", as in gaia).  We could certainly add something in the style sheet service for this.  Any sheet loaded from a chrome:// URI can style anonymous content, so it's just a matter of making sure it's only applied to the right documents.

In the meantime, it may be worth backing this patch out, since it changes the rendering of web content, per above...

Comment 11

[Myk Melez [:myk] [@mykmelez]]

(In reply to Boris Zbarsky (:bz) from comment #10)
> But there is no good infrastructure right now for styling only "content"
> (whatever that means in a world in which everything is actually "content",
> as in gaia).

The world in this case is actually B2G with Gaia running inside of it, and there's some chrome in B2G, which is why this bug exists.

In the Simulator, we currently work around the problem by loading the agent sheet into each content document on content-document-global-created, something like this:

Services.obs.addObserver(
  function(subject, topic, data) {
    subject.QueryInterface(Ci.nsIInterfaceRequestor).
            getInterface(Ci.nsIDOMWindowUtils).
            loadSheet(CONTENT_AGENT_SHEET, Ci.nsIDOMWindowUtils.AGENT_SHEET);
  },
  "content-document-global-created",
  true
);

Is this a reasonable workaround until we have platform support for doing this?

Comment 12

[Boris Zbarsky [:bzbarsky]]

> and there's some chrome in B2G, which is why this bug exists.

Sure.  My point is that it's not clear whether this bug means to apply the sheet to the gaia parts of the UI.

> Is this a reasonable workaround until we have platform support for doing this?

That synchronously reads the sheet from disk and parses it at every global creation.  Seems not great for performance.

Comment 13

[Myk Melez [:myk] [@mykmelez]]

(In reply to Boris Zbarsky (:bz) from comment #12)
> Sure.  My point is that it's not clear whether this bug means to apply the
> sheet to the gaia parts of the UI.

Ah, I see.  Yes, the intent here is to apply the sheet to Gaia parts of the UI in addition to third-party apps and web content.

Comment 14

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v2

Does making it this way makes sense?

Comment 15

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 755347 [details] [diff] [review]
Patch v2

"about://" does not.  This will also fail on jar: and ftp: and so forth...

Comment 16

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Backouted: https://hg.mozilla.org/mozilla-central/rev/43c0123f158b

Comment 17

[Adolfo Jayme]

This bug is no longer valid; I think it can be closed as WONTFIX, as well as the other dependencies of bug 943878.