Closed Bug 1223550 Opened 4 years ago Closed 4 years ago

Flash of unstyled content for selected tab (tab curve momentarily missing) on Yosemite with a clean profile

Categories

(Firefox :: Theme, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 47
Tracking Status
firefox44 --- unaffected
firefox45 + fixed
firefox46 --- fixed

People

(Reporter: bgrins, Unassigned)

References

Details

(Keywords: regression)

Attachments

(5 files)

I noticed this recently and did a regression test which narrowed it down to https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=b37f781fb5910e53a762f438e8643000ee0ef0ae&tochange=28e3554983f6af15685da76d7966b1a63e1d2fc0 AKA Bug 1088763.

So if you ./mach run or ./mach mochitest on Yosemite you should notice a brief time when the tab curves haven't loaded.  I will try to capture a screencast of the problem.
Attached image tab-flash-content.gif
screencast of issue on a debug build
I'm assuming this would be fixable by adding the yosemite bg tab files to the list of background pngs we preload elsewhere?
Also, is this just for debug builds? I haven't seen it on my machine, but maybe that's because I always build opt? :-)
(In reply to :Gijs Kruitbosch from comment #4)
> Also, is this just for debug builds? I haven't seen it on my machine, but
> maybe that's because I always build opt? :-)

I see it in opt builds but took the screencast in a debug build because it's more noticeable there.  I just confirmed also that I see it in a Nightly build on first startup.
It may also be more noticeable in e10s (hard for me to say - I can notice it in both modes)
I now see this some of the time, but adding the selected tab images to the list of preloaded images does not fix it.

I'm not really sure what else we can do here. Matt/Brian, do either of you have other ideas?
Flags: needinfo?(bgrinstead)
Flags: needinfo?(MattN+bmo)
Attached patch strawman patchSplinter Review
Still seeing this after each restart while my session is restoring. Needinfo'ing myself to try Gijs' patch.
Flags: needinfo?(mconley)
Duplicate of this bug: 1223843
I was talking to Mike about this - I wonder if we need to specify the px size of the caching hack because they're SVGs and otherwise they won't get cached correctly.
I applied this patch and still see the problem. Definitely willing to re-test when we try this with specific sizes.
Flags: needinfo?(mconley)
(In reply to Mike Conley (:mconley) - Needinfo me! from comment #12)
> I applied this patch and still see the problem. Definitely willing to
> re-test when we try this with specific sizes.

I tried this, it didn't help. I talked to Brian about this and he said he still had more ideas here and tried bisecting this. Brian, how far did you get with that?
This is what I mentioned last week.  If I take images out of the equation and just set background: red !important, etc for all of the rules then the same problem happens on startup.  When the window is unfocused after startup you see the colors you'd expect.  Gijs, I'm curious if you see the colors on startup with this patch applied (and I guess also with the Yosemite media query removed).

Note that if I set it to background-color instead the problem goes away (although you don't ever see the background color on startup).
Flags: needinfo?(bgrinstead)
(In reply to Brian Grinstead [:bgrins] from comment #14)
> Created attachment 8698175 [details] [diff] [review]
> debugging-styles.patch
> 
> This is what I mentioned last week.  If I take images out of the equation
> and just set background: red !important, etc for all of the rules then the
> same problem happens on startup.  When the window is unfocused after startup
> you see the colors you'd expect.  Gijs, I'm curious if you see the colors on
> startup with this patch applied (and I guess also with the Yosemite media
> query removed).
> 
> Note that if I set it to background-color instead the problem goes away
> (although you don't ever see the background color on startup).

I can test this but I expect I'd be seeing the same as you. This seems like a layout problem of sorts to me.

Daniel, I talked briefly about this to you during Mozlando. It now seems to not be related to SVG (I tried pngs, that didn't help, and Brian tried just overriding it with essentially "none" (implied by background: <color> ) in this patch)... any chance you can figure out why we're seeing this and/or how we could fix this "flash of unstyled content" issue? Or who might know - you originally pointed to Seth, but it seems like that might no longer make sense?

Markus, pretty much the same question - any idea what's going on here?
Flags: needinfo?(mstange)
Flags: needinfo?(dholbert)
I don't have cycles to dive into this in detail (and I don't normally develop on Mac, so I'd have some spinup time if I were to dive in).

I don't know what might be going on, but my main suggestion for making forward progress would be to try coming up with an extremely-minimal stripped-down version of the patch that caused this regression (linked in comment 0) -- the smallest version of that patch that's sufficient to trigger this bug, basically.  That will likely be helpful for isolating the issue.
Flags: needinfo?(dholbert)
Gentle poke - this is now on Aurora, and still looks pretty bad. Is it worth backing out the regressing patch on Aurora until we get this fixed on Nightly?
Flags: needinfo?(bgrinstead)
(In reply to Mike Conley (:mconley) - Needinfo me! from comment #17)
> Gentle poke - this is now on Aurora, and still looks pretty bad. Is it worth
> backing out the regressing patch on Aurora until we get this fixed on
> Nightly?

That makes sense to me, I'll leave it up to you / Gijs / shorlander.  The issue the regressing patch is fixing seems less noticeable than the issue it's causing so I'd vote yes.
Flags: needinfo?(bgrinstead)
So we're approaching another uplift, and this still hasn't been fixed. Can we just back out bug 1088763 entirely until we ensure it doesn't re-open this bug?
Flags: needinfo?(gijskruitbosch+bugs)
Sorry, I don't have any time to look into this at the moment either.
Flags: needinfo?(mstange)
(In reply to Mike Conley (:mconley) - Needinfo me! from comment #20)
> So we're approaching another uplift, and this still hasn't been fixed. Can
> we just back out bug 1088763 entirely until we ensure it doesn't re-open
> this bug?

I would be more comfortable with that if we knew what the bug actually *is* and had a path to fixing it. It is frustrating that despite efforts by several people we still have essentially 0 idea about what's going on here. I have already looked at it myself, and it seems unlikely to me that I myself will figure out a root cause, which means I will just have to unassign bug 1088763 and we'll have to leave that to rot instead, which doesn't seem like a great solution to the issue either.

Is there someone in the Toronto office who'd be able to look into the XUL layout side of this issue?
Flags: needinfo?(mconley)
Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(MattN+bmo)
Not in the Toronto office, but tn might be able to weigh in?
Flags: needinfo?(mconley) → needinfo?(tnikkel)
Setting a needinfo on myself. If we don't have actionable data on how to proceed on this regression, I'm going to backout bug 1088763 before the next uplift.
Flags: needinfo?(mconley)
I backed out bug 1088763 with https://hg.mozilla.org/integration/fx-team/rev/fd55305662594a886ca5bf3ad77f3afbe4d53034, so this should be fixed.
Status: NEW → RESOLVED
Closed: 4 years ago
Flags: needinfo?(mconley)
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
Yikes, I'm pretty sure I missed the merge cutoff, so this still affects 46.

[Tracking Requested - why for this release]:

I need to backout bug 1088763 from aurora, otherwise users on OS X Yosemite+ can see an ugly flash of unstyled tab curves during session restore.
Target Milestone: Firefox 46 → Firefox 47
(In reply to Mike Conley (:mconley) - Needinfo me! from comment #26)
> Yikes, I'm pretty sure I missed the merge cutoff, so this still affects 46.
> 
> [Tracking Requested - why for this release]:
> 
> I need to backout bug 1088763 from aurora, otherwise users on OS X Yosemite+
> can see an ugly flash of unstyled tab curves during session restore.

Note that we don't actually turn aurora updates back on until the end of the week.

I think my commit hook for css-only approvals hasn't been enabled (I need to file a bug on that), but I think you're pretty much OK to land with "a=backout,css-only patch" on aurora once the merge has happened.
[bugday-20160323]

Status: RESOLVED,FIXED -> UNVERIFIED

Comments:
STR: Not clear.
Developer specific testing

Component: 
Name			Firefox
Version			46.0b9
Build ID		20160322075646
Update Channel	        beta
User Agent 		Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS			Windows 7 SP1 x86_64

Expected Results: 
Developer specific testing

Actual Results: 
As expected
Changing the initial value of mIsActive (which is used to evaluate the moz-window-inactive pseudoclass) to true fixes the problem for me locally. (I applied the patch from bug 1088763.)

This change is probably not the correct fix, but might help someone more knowledgeable than me about what happens during startup to see what is going wrong when these CSS rules are present (see comment 14).
Attachment #8747463 - Flags: feedback?(mconley)
Thanks aleth. I don't think I'm the one to look at this. I don't mean to put more load on mstange, but perhaps he has some ideas about why this is making a difference (I ask due to bug 508482).
Flags: needinfo?(mstange)
Comment on attachment 8747463 [details] [diff] [review]
Change initial value of mIsActive to true

Clearing feedback request until we hear more.
Attachment #8747463 - Flags: feedback?(mconley)
Attachment #8747463 - Flags: feedback?(mstange)
I want to look into what's happening here, but I haven't gotten a chance to do so yet. Hopefully this week.
The dark inactive window appearance is really bothering me too.
Success!

The preloading hack that (In reply to :Gijs Kruitbosch from comment #7)
> I now see this some of the time, but adding the selected tab images to the
> list of preloaded images does not fix it.

The problem was that you only added the inactive images to the preloading list. You also need to add all the active ones, because they get overwritten with the inactive ones while the window is loading, so there's nothing else to start loads for them. Once the window is shown, it becomes active and wants to paint the active images, so those need to be available.
Flags: needinfo?(tnikkel)
Flags: needinfo?(mstange)
Attachment #8747463 - Flags: feedback?(mstange)
The next time an issue like this comes up, you can use this patch to see what URLs aren't loading until after the window is shown.
You need to log in before you can comment on or make changes to this bug.