1671619 - Wiki page flickers when the user switches between this and Customize Firefox page

Status: RESOLVED DUPLICATE of bug 1842809

(Core :: Layout: Images, Video, and HTML Frames, defect)

Description

[Catalin Sasca, Desktop Test Engineering [:csasca]]

Affected versions

• Firefox 82.0
• Firefox 83.0a1

Affected platforms

• macOS 11
• Windows 10
• Ubuntu 20.04

Steps to reproduce

1. Launch Firefox
2. In one tab access for example this wiki page
3. In another tab have Customize page opened
4. Switch between the two tabs

Expected result

• The wiki page stays rendered

Actual result

• The page flickers twice

Regression range

• I will see for a regression

Additional notes

• The issue can be seen in the following attachment

Suggested severity

• S3

Comment 1

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=5cff72743ebc9266a78e4c6a41d17d68ac651326&tochange=5b1724bafc9387044cc794e9f69c31f151b26fd0

Regressed by Bug 1558995

Comment 2

[Boris Chiou [:boris]]

Hi Brian, would you please take a look at this because this is regressed by Bug 1558995? Thanks.

Comment 3

[Brian Grinstead [:bgrins]]

Attachment: flicker-with-browsertoolbox-open-2.gif

This gif does show a bit of a flicker, but not as much as the video attached in Comment 0 which shows the web content immediately render and then flicker and then re-render.

I confirmed the attributes are being toggled in the frontend at the same time (collapsed for the #browser element and hidden for the #customization-container). This happens at https://searchfox.org/mozilla-central/rev/819be4899a92213abf121b449779ced662f2ce13/browser/components/customizableui/CustomizeMode.jsm#401-404 for entering Customize Mode and https://searchfox.org/mozilla-central/rev/819be4899a92213abf121b449779ced662f2ce13/browser/components/customizableui/CustomizeMode.jsm#530-533 for exiting.

:kats - I know there was some document splitting related changes for Customize Mode in Bug 1559389 as a result of Bug 1558995 (which caused this regression here). Do you think there's any relation here, or totally new problem?

Comment 4

[Brian Grinstead [:bgrins]]

Here's a profile for the same STR https://share.firefox.dev/34bT4YT

Comment 5

[Brian Grinstead [:bgrins]]

Attachment: flicker.gif

Here's a better gif of the problem showing the double render (I think it may have started happening once I made the window much larger than when I recorded the gif with the Browser Toolbox open)

Comment 6

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

The document splitting code has since been removed and all the functional changes should have been removed. I would be surprised if a change related to document splitting turned up here as being relevant, unless we missed something when deleting the code. So my guess here is that it's a totally new problem.

Comment 7

[Brian Grinstead [:bgrins]]

Here's a profile with the double render https://share.firefox.dev/3o0QnS1

Comment 8

[Brian Grinstead [:bgrins]]

Best guess based on a quick chat with :dholbert is that the style system is re-resolving styles when the tab goes from active to inactive, which ends up causing a repaint.

I'm at a bit of a loss about the blank paint in the middle though. Emilio - do you have any ideas? Comment 5 and Comment 7 are the most relevant bits here.

Comment 9

[Brian Grinstead [:bgrins]]

A couple more notes from our chat:

the page has a visibilitychanged listener on the root node (along with a bunch of others, but I think that's the only one that fires on tabswitch)
some fraction of the time, when I trigger the bug, I can also see the "Read|View Source|View History|" tabs "sliding in" from the right (edited)
and in devtools, it looks like that's happening via them dynamically setting the style attribute on those nodes (it's not like a pure-CSS-no-JS sort of thing)
it feels likely that this is related to the visibilitychange event and whatever JS they do as a result

Comment 10

[Emilio Cobos Álvarez (:emilio)]

So the issue is that bug 1558995 removed the <deck> (which keeps the dimensions of the panel that's hidden) and changed it by .collapsed = true, which resizes the <browser> to be zero-size and thus has a bunch of side effects.

For example, if you put the devtools for the wikipedia tab in a separate window, then switch to the customizableui tab, you can see that window.innerHeight and window.innerWidth become 0 in the wikipedia tab. If I try the same with mozregression --launch 68 I see that innerWidth etc do not change.

So I suspect the slide-in thing is not about visibilitychange (this happens regardless), it is about resize events. This probably also explains the blank paint and so on.

Does that help?

Comment 11

[Brian Grinstead [:bgrins]]

Yes - let me check if using .hidden makes a difference. I believe the .collapsed is carryover from the XBL days where hiding the element would destroy the binding and I can't think of a reason to prefer it here.

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Quite likely .hidden has the same issue.

Comment 13

[Brian Grinstead [:bgrins]]

FWIW switching to .hidden seems to fix the problem locally. I see an instant switch between Customize Mode and the wiki page with no repaint.

Comment 14

[Brian Grinstead [:bgrins]]

Attachment: Bug 1671619 - Use [hidden] instead of [collapsed] for the browser container when entering and exiting Customize Mode

This is using collapsed as a carryover from XBL days, where setting the element to hidden would destroy bindings.
But it's causing a side effect of shrinking the browser element to 0x0 which causes the content to resize, leading to
some flickering and poor performance when switching back to content from a Customize Mode tab.
By setting hidden we instead are just toggling the display property.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/38627c09131d
Use [hidden] instead of [collapsed] for the browser container when entering and exiting Customize Mode r=Gijs

Comment 16

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/38627c09131d

Comment 17

[Sandor Molnar[:smolnar]]

Backed out for causing bug 1672560

https://hg.mozilla.org/mozilla-central/rev/7e2acd77c07fc53754dbc685f3323e9055b82060

Comment 18

[Brian Grinstead [:bgrins]]

Emilio, should we focus on fixing the root cause of Bug 1672560 for L10nMutations and reland this or do you think it's just too risky to set the browser elements to hidden?

If the latter I guess we need to re-add a <deck> or some other parent that will allow the children to keep full height even when not visible. It'd be a good test for if Bug 1559192 will preserve that behavior if/when we migrate deck away from nsDeckFrame and to CSS grid (i.e. apply the patch over there on top of the deck addition and make sure we don't have any customize mode problems).

Comment 19

[Emilio Cobos Álvarez (:emilio)]

Honestly, I think the risk comes mostly for non-e10s tabs. Bug 1672560 probably just needs an extra check.

Is visibility: hidden usable for this or such?

Comment 20

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:bgrins, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 21

[Mathew Hodson]

I think this was filed under the wrong bug component. It should probably be under Firefox :: Toolbars and Customization.

Comment 22

[Gregory Pappas [:gregp]]

Sorry for not searching before filing bug 1842809.

It looks like bug 1687592 will prevent bug 1672560-like crashes but if not, we might have to back out bug 1842809 :(