Closed Bug 759678 Opened 10 years ago Closed 10 years ago

Flash of unstyled content in Fennec in this case

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

(firefox14 verified, firefox15 verified, firefox16 verified, blocking-fennec1.0 -, fennec15+)

VERIFIED FIXED
Firefox 16
Tracking Status
firefox14 --- verified
firefox15 --- verified
firefox16 --- verified
blocking-fennec1.0 --- -
fennec 15+ ---

People

(Reporter: martijn.martijn, Assigned: kats)

References

()

Details

(Keywords: testcase)

Attachments

(2 files)

This is basically like bug 721523, but filed against Fennec.

See testcase, in Fennec the page looks red for a moment, which does not happen in Firefox desktop.
However, I'm also seeing it on Google Chrome beta on Android.

The stylestheet used in the testcase has to be of a certain size to get the bug reproduced (so I put just a lot of junk in it and the green background rule at the end)

When the meta viewport tag is removed:
    <meta name="viewport" content="width=device-width, user-scalable=no, maximum-scale=1">
I can't reproduce the bug anymore in Fennec (in Google Chrome beta on Android, I'm also seeing the bug there).

I guess this bug might occur because of the interaction with the meta viewport tag?
Bug 721523 (and the duped bug 755865) are blocking-Fennec1.0+, so this bug should too.
blocking-fennec1.0: --- → ?
blocking-fennec1.0: ? → -
AMO says this isn't reproducible in Fx12, can we confirm that? And get a regression range if so.
blocking-fennec1.0: - → ?
Odd. Depends on a slow load whether you see red or green.  I did it with the phone network and it stayed red.  When I tried with wi-fi I got mixed red then turning to green.  If the page is in cache, it doesn't necessarily flash red at all.  (tested on Nightly, 5/31/2012 on Galaxy Nexus)
Happens in Fennec 12 nightly native, 2012-01-03 http://hg.mozilla.org/mozilla-central/rev/44d992ccc97a

Happens in Fennec 11 aurora native, http://hg.mozilla.org/releases/mozilla-aurora/rev/e44cc659b759

Seems to work fine in Fennec 10 Nightly XUL, 11/02/2011 buildconfig : http://hg.mozilla.org/mozilla-central/rev/978002c0b0ad
I suspect this change, which altered the content.sink.* preferences:
http://hg.mozilla.org/projects/birch/rev/98861dbb3277

We could try reverting those prefs in about:config.
In the nightly 5/31/2012, I changed the perfs to match the values in: 
http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsContentSink.cpp#159

It made the red more visible.... it wasn't just a flash, it was red, pause, green.
If you're seeing a flash, that means something is starting layout on the page before the stylesheet is done loading.

The most likely reason for that is the Fennec UI asking for layout information sometime during the load process before we've even started layout.

If someone who has a debug fennec build can reproduce this, a stack to PresShell::InitialReflow would probably give us at least some idea of what's triggering the layout start.
tracking-fennec: --- → 15+
blocking-fennec1.0: ? → -
Here's the backtrace. The event being dispatched at the bottom of the stack is the onDOMMetaAdded event, and I assume the script that requests scrollWidth is the DOMMetaAdded listener in browser.js
Yeah.  So.... don't do that.  ;)

We can expose a getter for the "layout started" state if that would be useful...
Attached patch PatchSplinter Review
This makes us skip processing DOMMetaAdded events that arrive between the onLocationChange (the start of a page load) and the before-first-paint event (when the page is about to get displayed). Note that the before-first-paint handler runs updateMetadata, so any DOMMetaAdded events during this period are effectively batch-processed once just before the page is displayed.
Attachment #631964 - Flags: review?(mbrubeck)
Assignee: nobody → bugmail.mozilla
Attachment #631964 - Flags: review?(mbrubeck) → review+
Landed on inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce4fcd70f3c9

Note that without bug 763592 the "flash of unstyled content" will still be there when *reloading* the page, as that scenario is incorrectly detected as a same-document page load. Navigating to a page for the first time, or from some other page, should be fixed with just this patch.
Target Milestone: --- → Firefox 16
https://hg.mozilla.org/mozilla-central/rev/ce4fcd70f3c9
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment on attachment 631964 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: on pages with a meta viewport, an extra reflow is done during page load, which could result in rendering some half-baked document (because other information in the <head> may not have been processed yet). the extra reflow could also have a negative performance impact.
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): mobile-only, pretty low risk.
String or UUID changes made by this patch: none

note that this patch depends on the patch from bug 763592 to work properly on page reloads.
Attachment #631964 - Flags: approval-mozilla-beta?
Attachment #631964 - Flags: approval-mozilla-aurora?
(In reply to Kartikaya Gupta (:kats) from comment #14)
> Comment on attachment 631964 [details] [diff] [review]
> Patch
> 
> [Approval Request Comment]
> Bug caused by (feature/regressing bug #): 
> User impact if declined: on pages with a meta viewport, an extra reflow is
> done during page load, which could result in rendering some half-baked
> document (because other information in the <head> may not have been
> processed yet). the extra reflow could also have a negative performance
> impact.
> Testing completed (on m-c, etc.): on m-c
> Risk to taking this patch (and alternatives if risky): mobile-only, pretty
> low risk.
> String or UUID changes made by this patch: none
> 
> note that this patch depends on the patch from bug 763592 to work properly
> on page reloads.

When you're considering taking this patch, you should know we have a very hacky workaround on AMO to make our pages load without the flicker which I almost r-'d and I'm still considering pulling if it starts using up developer time to deal with.  We definitely need this in ASAP if you want a good mobile add-ons experience.
Comment on attachment 631964 [details] [diff] [review]
Patch

Discussed in triage with kats - approved for b7. Go go go!
Attachment #631964 - Flags: approval-mozilla-beta?
Attachment #631964 - Flags: approval-mozilla-beta+
Attachment #631964 - Flags: approval-mozilla-aurora?
Attachment #631964 - Flags: approval-mozilla-aurora+
verified on Firefox 14 beta 7 build 2
The red background does not flash when loading/reloading the testcase page.

Verified fixed on:
Aurora 15.0a2 2012-07-10/Nightly 16.0a1 2012-07-10/Firefox Mobile 14.0b11
HTC Desire
Android 2.2.2
Status: RESOLVED → VERIFIED
Depends on: 771575
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.