1193668 - e10s loading screen is broken when lightweight theme is used

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Attachment: lwt-e10s.png

When the content process isn't ready after tab switching and a lightweight theme is in use, the loading screen for the content process is broken.

It should just show the spinner on a light gray background as if no theme was being used.

Comment 1

[:Felipe Gomes (needinfo for replies!)]

This was caused by the opacity: 0.3 from bug 1106527.

Phillip, the opacity: 0.3 as added in that patch does not mean the same thing as from your mock-up. I believe your goal was just to make the spinner more faded-out, but in practice it had the effect of fading out the entire browser area.

I could change the structure a bit and move the opacity: 0.3 just to the spinner, but opacity slows things down so it's better if we just generate yet another spinner that is solid but with the resulting faded out colors.

On a side note: bug 1106527 had the (unintended?) side effect of making the entire spinner page gray-ish instead of pure white (at least on non-lwtheme mac, as the background of the window is gray), which I thought was kinda nice and less jarring. But I believe from your mockup that the intention was indeed #fff. When I remove the opacity it will go back to being #fff unless we change the background color. Do you confirm this is what is wanted?  (I can generate screenshots/try builds if you want to play with it)

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Note: bug 1191909 was flagging that the opacity caused such a perf regression on Linux that I couldn't load my main profile until I cut it down to ~ 3 windows (from ~24).

Comment 3

[:Felipe Gomes (needinfo for replies!)]

Attachment: Remove opacity

Just want to get this main part of the patch reviewed before mconley goes on PTO.

The other part of the patch would be to possibly change the spinner image and/or background-color, but I believe a ui-r+ from Philipp suffices for that.

I also want to profile if `visibility: hidden` is better than `opacity: 0` for the browser element, but I'll leave that to a separate bug. And perhaps that will be less important after bug 1180916.

Comment 4

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Sure, we can put the opacity directly into the spinner!
As for the background: the gray was an accident, but a happy one :)
If we remove the opacity, we should change the background color to a light gray (probably something like #eee)

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8648182 [details] [diff] [review]
Remove opacity

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

Yep, this is the right move. I'm also kinda bummed I didn't think about the performance impact of doing such a giant transparency. :/

Thanks felipe!

Comment 6

[:Felipe Gomes (needinfo for replies!)]

Attachment: Remove opacity and change bg-color

So just changing the background-color doesn't work. We gotta use !important because there's a bg-color directly set as a style attribute on tabbrowser, here: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/tabbrowser.xml?rev=4c7127a6ae54#4084

Also, for a better hi-dpi img we are changing the spinner file to be 60x60

Comment 7

[:Felipe Gomes (needinfo for replies!)]

Documenting how the spinner was generated, in case we need to adjust it later:

- The spinner now doesn't have transparency. It was generated on preloaders.net with the colors params:
  - background color: #f9f9f9
  - foreground color: #b0b0b0
- Generated at 120x120 and downloaded the sprites
- Resized to 60x60
- Separated each frame to a separate file
- generated the apng through http://littlesvr.ca/apng/assembler/assembler.php

This tool gives a much better apng file size than preloaders.net, so that's one reason phlsa and I were ok with shipping it at 60x60.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/3658808a325a

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3658808a325a