Closed
Bug 1067173
Opened 10 years ago
Closed 10 years ago
The page with Tiles is not rendered correctly
Categories
(Firefox :: New Tab Page, defect)
Tracking
()
People
(Reporter: mkem, Assigned: ttaubert)
References
Details
(Keywords: regression)
Attachments
(3 files)
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
patch
|
adw
:
review+
Mardak
:
feedback+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:33.0) Gecko/20100101 Firefox/33.0
Build ID: 20140911191954
Steps to reproduce:
1. we have to have set up home page to "about:newtab"
2. open page www.abclinuxu.cz or www.slovnik.cz or speedtest.net
3. click on home icon in toolbar menu
4. new page with tiles is shown, but it is not rendered properly. I attached the screenshot with correct rendering and incorrect rendering
Actual results:
After the steps described upper the page with Tiles is not rendered correctly. Strange is that it is not happened with every page. I was able to reproduce it with pages meant upper. Maybe it has something together with that previews of problematic pages are also in tiles.
Expected results:
Every time correct rendering of page with Tiles
Additional info: If I have incorrect rendered page with Tiles and change the size of window, nothing happens. I also made "reset Firefox to its default state". But the results are the same.
I just tested the problem in Firefox 32.0.1 and the problem with rendering of page with Tiles didn't happen.
Comment 4•10 years ago
|
||
When you get such a broken new tab page, if you open the browser console (Tools > Web Developer > Browser Console) are there any errors that seem relevant?
Severity: normal → critical
Component: Untriaged → New Tab Page
Flags: needinfo?(mkem)
Keywords: regression,
regressionwindow-wanted
I checked via the console and find only this message:
Error during parsing value "max-height". Declaration was omitted.
Nothing else. When I have broken newtab page and click again on icon "home". I get the newtab page correctly rendered.
Flags: needinfo?(mkem)
Comment 6•10 years ago
|
||
Do you have any custom values for browser.newtab* in particular browser.newtab.preload ?
All properties of browser.newtab* are set to default. Only these ones are set to the following values.
browser.newtabpage.enhanced;true
browser.newtabpage.storageVersion;1
Assignee | ||
Updated•10 years ago
|
Severity: critical → normal
Assignee | ||
Comment 8•10 years ago
|
||
I can't reproduce the issue with the given STR but was able to with some fiddling around in the code. My guess is that we somehow end up calling _resizeGrid() earlier than expected, before the document has finished loading. This leads to a negative max-height value and yields the given error message. Should be safe to just bail out in that case and wait for the load event to call _resizeGrid() again.
Assignee: nobody → ttaubert
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #8492663 -
Flags: review?(edilee)
Comment 9•10 years ago
|
||
Comment on attachment 8492663 [details] [diff] [review]
0002-Bug-1067173-Bail-out-early-if-_resizeGrid-is-called-.patch
>diff --git a/browser/base/content/newtab/grid.js b/browser/base/content/newtab/grid.js
>+++ b/browser/base/content/newtab/grid.js
> _resizeGrid: function Grid_resizeGrid() {
>+ if (document.readyState != "complete") {
>+ return;
The codepaths triggering _refreshGrid should be from:
1) Page_update -> Grid_refresh
2) Grid_handleEvent(load)
3) Grid_handleEvent(resize)
It shouldn't be (2) because that should only happen when document.readyState == "complete" so something is triggering an update each time or the window is being resized? Either way, we don't want to try to read out sizes until things have flowed.
I can't give r+, but looks good to me!
Attachment #8492663 -
Flags: review?(edilee)
Attachment #8492663 -
Flags: review?(adw)
Attachment #8492663 -
Flags: feedback+
Comment 10•10 years ago
|
||
Comment on attachment 8492663 [details] [diff] [review]
0002-Bug-1067173-Bail-out-early-if-_resizeGrid-is-called-.patch
Review of attachment 8492663 [details] [diff] [review]:
-----------------------------------------------------------------
This is a worthwhile precaution, but since it's not clear what's causing the bug I'm not convinced this will fix it -- but probably none of us are. :-) Only thing is that this might end up masking some deeper problem.
Of Ed's three code paths, (1) seems most likely to me, like browser history changes, triggering an update for hidden pages, and then a newtab is preloaded but before it finishes loading its update() is called. Maybe gAllPages.register(this) in gPage.init should be moved to a load listener?
Attachment #8492663 -
Flags: review?(adw) → review+
Assignee | ||
Comment 11•10 years ago
|
||
(In reply to Drew Willcoxon :adw from comment #10)
> This is a worthwhile precaution, but since it's not clear what's causing the
> bug I'm not convinced this will fix it -- but probably none of us are. :-)
Indeed.
> Only thing is that this might end up masking some deeper problem.
I'm not too worried about that... I will land it and keep thinking about a better approach to this. We can also talk about this in person next week and see if we can come up with something nicer. In any case we'd have a small patch we could easily uplift to 33?
> Of Ed's three code paths, (1) seems most likely to me, like browser history
> changes, triggering an update for hidden pages, and then a newtab is
> preloaded but before it finishes loading its update() is called.
Page.upate() could call it when populateCache() has already finished, yeah. The "resize" event handler could probably also be called when we don't expect it.
> Maybe gAllPages.register(this) in gPage.init should be moved to a load listener?
I'd rather not do this as it would defer rendering the page a bit. But let's talk next week?
Assignee | ||
Comment 12•10 years ago
|
||
[Tracking Requested - why for this release]:
This issue has been reported for Firefox 33 but isn't easily reproducible unfortunately. It very visibly breaks the new tab page - recovering seems rather easy but I assume that's only true for the more experienced users.
status-firefox32:
--- → unaffected
status-firefox33:
--- → affected
status-firefox34:
--- → affected
status-firefox35:
--- → affected
tracking-firefox33:
--- → ?
tracking-firefox34:
--- → ?
Assignee | ||
Comment 13•10 years ago
|
||
Assignee | ||
Comment 14•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Assignee | ||
Updated•10 years ago
|
Updated•10 years ago
|
Assignee | ||
Comment 16•10 years ago
|
||
Sure, just wanted to wait for any possible regression reports. Haven't seen any.
Flags: needinfo?(ttaubert)
Assignee | ||
Comment 17•10 years ago
|
||
Comment on attachment 8492663 [details] [diff] [review]
0002-Bug-1067173-Bail-out-early-if-_resizeGrid-is-called-.patch
Approval Request Comment
[Feature/regressing bug #]: ?
[User impact if declined]: Newtab page can break visibly, showing only a single cut-off tile. Recovery possible but may leave some users clueless.
[Describe test coverage new/current, TBPL]: No additional test because the original issue is hard to reproduce.
[Risks and why]: Risk seems low, we will simply not attempt to resize the grid if the page isn't ready yet.
[String/UUID change made/needed]: None
Attachment #8492663 -
Flags: approval-mozilla-beta?
Attachment #8492663 -
Flags: approval-mozilla-aurora?
Updated•10 years ago
|
Attachment #8492663 -
Flags: approval-mozilla-beta?
Attachment #8492663 -
Flags: approval-mozilla-beta+
Attachment #8492663 -
Flags: approval-mozilla-aurora?
Attachment #8492663 -
Flags: approval-mozilla-aurora+
Comment 18•10 years ago
|
||
Comment 19•10 years ago
|
||
Michal, can you please confirm whether this is fixed in:
- Firefox 34 Beta 1 - ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/34.0b1-candidates/build2/
or
- latest Firefox Aurora - ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-aurora/
or
- latest Firefox Nightly - ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-central/
Thank you in advance!
Flags: needinfo?(mkem)
Reporter | ||
Comment 20•10 years ago
|
||
Hello, the bug disappeared in the last 2 betas of Firefox 33. Now I'm using Firefox 34b1 and it also looks great and works without any flaw.
Just for assurance I tested the versions, you posted in your comment, and no issue appeared. Everything works flawless. The posted versions were tested on OS X 10.10 fresh installed.
Thank you for your attention and quick fix of this bug. Michal.
Flags: needinfo?(mkem)
Comment 21•10 years ago
|
||
Verified by reporter per comment 20. Thanks Michal!
Status: RESOLVED → VERIFIED
Flags: firefox-backlog+
Updated•10 years ago
|
Iteration: --- → 36.1
Flags: qe-verify+
Updated•10 years ago
|
Points: --- → 3
Updated•10 years ago
|
Keywords: regressionwindow-wanted
You need to log in
before you can comment on or make changes to this bug.
Description
•