User-Agent: Build Identifier: Mozilla/5.0 (X11; Linux x86_64; rv:2.0b10pre) Gecko/20110114 Firefox/4.0b10pre A browser window may keep a state which causes late application of external CSS. Initially the content is rendered without CSS and shortly thereafter the CSS is applied. This appears as flicker. When you pull the tab out creating a new browser window the error vansishes. If you put the tab back oder otherwise invoke the URL in the first window and reload the error is reproduced. Reproducible: Always Steps to Reproduce: 1. open http://hg.mozilla.org/mozilla-central 2. etrl-e (enter panorama) 3. shrink tab group, pull tab out of tab group (-> tab group vanishes) 4. click at tab 5. reload (ctrl-r or mouse on reload) Actual Results: Page is renderered without CSS initially then CSS is applied. Expected Results: Page is rendered with CSS.

reduced STR: 1. ctrl-e (-> panorama) 2. ctrl-e (back to normal view) 3. open http://hg.mozilla.org/mozilla-central

The first bad revision is: changeset: 59796:abc78094ad64 user: Mounir Lamouri <mounir.lamouri@gmail.com> date: Mon Jan 03 13:57:45 2011 +0100 summary: Bug 569399 - autofocus sometimes does not work because the element has no frame yet. r=hsivonen a=blocking-betaN

Product: Core Component: HTML: Parser According to Mike Beltzner's remark in Bug 609396 I would like to set the "Component" accordingly but I can't: Theres no option named "HTML: Parser".

There was no such option in the original drop-down element ...

Repro'd with Mozilla/5.0 (Windows NT 5.1; rv:2.0b10pre) Gecko/20110114 Firefox/4.0b10pre ID:20110114030359 Panorama seems to be an easy Way to trigger this Bug, but I've seen this myself "in the Wild" without using it as well.

This has fixed the problem: author Sean Dunn <seanedunn@yahoo.com> Tue Nov 16 16:35:00 2010 -0600 (at Tue Nov 16 16:35:00 2010 -0600) changeset 60722 99bae22d8c8c parent 60721 f2091ca7dc39 pushlog: 99bae22d8c8c Bug 609685 - "Having opened panorama once in a window affects all rendering in that window, even in tabs that are opened later" [f=ian r=ian a=blocking]

Indeed. If panorama requests layout even when we don't have all of our style... we have to do layout. Even though we don't have all the styles yet. Good thing they stopped doing that.

The issue is back again. Just checking with nightly build.

2011-02-03-09-mozilla-central is okay, but the error reappears with The first bad revision is: changeset: 61867:54da8f75df7f user: Tim Taubert <tim.taubert@gmx.de> date: Thu Feb 03 02:53:13 2011 +0100 summary: Bug 594958 - Tab thumbnails sometimes have gray (now black) bars at the bottom [r=ian, a=blocking2.0:final+] The issue can be reproduced with each of patch v4, v5, v6 and of course with the patch for checkin. This is remarkable since until a few hours ago I have been using a version of 2011-01-31 (ba3fe7ee56b9 + v5/v6 of his that patch) which does not show the issue.

Over to Tim, as this is blamed on his recent changes.

Alternatively one may back out changeset: 61680:d94a56d78123 user: Patrick Walton <pwalton@mozilla.com> date: Mon Jan 31 16:41:05 2011 -0800 summary: Bug 624931 - Use -moz-transform for Panorama zoom. r=iangilman a=sdwilsh These changes are conflicting with those of 61867:54da8f75df7f.

This started happening again because the code from bug 594958 is flushing layout on the content window. It's still passing DO_NO_FLUSH to the drawWindow call, so I assume it doesn't _mean_ to flush. But _calculateClippingRect requests the scroll* properties of the body, which triggers a layout flush of course. (I'm also not sure where the magic "25" adjustment to the width comes from, but that's a separate issue.)

(In reply to comment #13) > This started happening again because the code from bug 594958 is flushing > layout on the content window. It's still passing DO_NO_FLUSH to the drawWindow > call, so I assume it doesn't _mean_ to flush. But _calculateClippingRect > requests the scroll* properties of the body, which triggers a layout flush of > course. Ok that's bad. I wasn't aware of this bug when I "fixed" bug 594958. > (I'm also not sure where the magic "25" adjustment to the width comes from, but > that's a separate issue.) That's the vertical scrollbar. Loosing some pixels on right does not hurt that much so that's a bit easier than getting the documentElement and its clientWidth/Height (we also don't want the documentElement - we want the window size as that is what the user sees).

> That's the vertical scrollbar. Nothing guarantees that 25 is larger than the vertical scrollbar width, for what it's worth (and in fact on Windows with wide scrollbars I'll bet money it's not). For the rest, what's the code trying to measure? That is, why do you need the |if (body)| codepath at all, instead of just using the window's innerWidth/innerHeight?

This seems worse than the issue fixed by bug 594958, so if this can't be sorted out quickly, that patch should probably be backed out.

(In reply to comment #15) > Nothing guarantees that 25 is larger than the vertical scrollbar width, for > what it's worth (and in fact on Windows with wide scrollbars I'll bet money > it's not). Mhh true :/ We should correctly calculate the scrollbar width and subtract that from the window's inner width. > For the rest, what's the code trying to measure? That is, why do you need the > |if (body)| codepath at all, instead of just using the window's > innerWidth/innerHeight? Often the body's scroll size is greater than the viewport. So we want the window size at the minimum. If the page is greater than that we want the body's scroll size.

(In reply to comment #13) > This started happening again because the code from bug 594958 is flushing > layout on the content window. It's still passing DO_NO_FLUSH to the drawWindow > call, so I assume it doesn't _mean_ to flush. But _calculateClippingRect > requests the scroll* properties of the body, which triggers a layout flush of > course. So, is there any other way to determine a page's bounds? Or do we need to wait until the page reaches some state ("load")?

> We should correctly calculate the scrollbar width Or just ask Gecko for it, as a followup. > is there any other way to determine a page's bounds? Could add one in windowutils. Doesn't look like there is one now, offhand. roc?

Seems there's an easy fix for this. I'll file a follow-up for the scrollbar width issue.

(In reply to comment #20) > Created attachment 509767 [details] [diff] [review] > patch v1 > > Seems there's an easy fix for this. I'll file a follow-up for the scrollbar > width issue. So it's okay to flush layout if the page has loaded?

It won't cause the FOUC issue this bug is about. It might impact performance, depending on how it interleaves with the refresh driver and with page scripts.

(In reply to comment #22) > It might impact performance, depending on how it interleaves with the refresh > driver and with page scripts. Ok, sorry I didn't know this could have so much side-effects. So we need another follow up bug for getting page bounds without impacting on the content window?

Comment on attachment 509767 [details] [diff] [review] patch v1 > _update: function TabItems__update(tab) { > try { > if (this._pauseUpdateForTest) > return; > > Utils.assertThrow(tab, "tab"); > >+ // If the tab hasn't loaded completely, skip this update. >+ if (tab.linkedBrowser.contentDocument.readyState != 'complete') >+ return; It's interesting that _update gets called here at all. Seems like this should be prevented. Apart from that, I think we need a patch that makes this code not touch document.body, given comment 22.

(In reply to comment #23) > Ok, sorry I didn't know this could have so much side-effects. So we need > another follow up bug for getting page bounds without impacting on the content > window? Please no other followup. If it's an easy fix, it can just be done here. If it's not an easy fix, we need to revert the patch from bug 594958.

patch v1 breaks the smooth opening of new tabs wich have been created in panorama.

(In reply to comment #25) > Please no other followup. If it's an easy fix, it can just be done here. If > it's not an easy fix, we need to revert the patch from bug 594958. Ok, there seems no easy way of retrieving the actual page size for now. There are many other issues with tab thumbnail generation and we will punt them all after fx4. I reverted the patch and removed the whole part that accesses the document's body (so no FOUC and no performance impact). We're having pretty good results anyway. I filed bug 631593 and added a comment to replace the 25px static scrollbar width. That's better than removing it completely as that should be ok on most systems.

v2 looks good!

Corrected/adapted test for bug 594958.

(In reply to comment #24) > It's interesting that _update gets called here at all. Seems like this should > be prevented. This might be handled in a separate bug, but do you know what's going on here yet? Why is the _update called so aggressively during page load when panorama isn't open?

(In reply to comment #30) > This might be handled in a separate bug, but do you know what's going on here > yet? Why is the _update called so aggressively during page load when panorama > isn't open? This is done because we listen on TabAttrModified. TabItems._update() handles the thumbnail, favicon, url (stored internally) and label - so any of them could have been modified. We should probably split that up and handle thumbnail updates separately. This could be done together with bug 598435 that suggests to use MozAfterPaint.

Using TabAttrModified to update tab thumbnails /while panorama isn't open/ sounds way too aggressive and wasteful to me (MozAfterPaint seems even worse).

Note that this was actually discussed somewhat in bug 609685 comment 2. That's when the DO_NOT_FLUSH was added to the drawWindow call.

(In reply to comment #32) > Using TabAttrModified to update tab thumbnails /while panorama isn't open/ > sounds way too aggressive and wasteful to me (MozAfterPaint seems even worse). We receive the events when the user is not in the Panorama UI, but we simply set dirty flags; it's not till you reenter the Panorama UI that thumbnails are actually updated. If, for some reason, we are actually updating thumbnails while not in the Panorama UI, that's definitely a problem.

(In reply to comment #34) > (In reply to comment #32) > > Using TabAttrModified to update tab thumbnails /while panorama isn't open/ > > sounds way too aggressive and wasteful to me (MozAfterPaint seems even worse). > > We receive the events when the user is not in the Panorama UI, but we simply > set dirty flags; it's not till you reenter the Panorama UI that thumbnails are > actually updated. If, for some reason, we are actually updating thumbnails > while not in the Panorama UI, that's definitely a problem. This bug seems to indicate that this doesn't work as expected.

(In reply to comment #35) > > We receive the events when the user is not in the Panorama UI, but we simply > > set dirty flags; it's not till you reenter the Panorama UI that thumbnails are > > actually updated. If, for some reason, we are actually updating thumbnails > > while not in the Panorama UI, that's definitely a problem. > > This bug seems to indicate that this doesn't work as expected. Indeed. Filed bug 631662 to investigate.

Comment on attachment 509851 [details] [diff] [review] patch v3 (test corrected) Looks good.

(In reply to comment #29) > Created attachment 509851 [details] [diff] [review] > patch v3 (test corrected) Passed try: http://tbpl.mozilla.org/?tree=MozillaTry&pusher=tim.taubert@gmx.de&rev=4324e8f07936

Verified on Mozilla/5.0 (Windows NT 6.1; rv:2.0b12pre) Gecko/20110206 Firefox/4.0b12pre