Closed
Bug 1059591
Opened 9 years ago
Closed 9 years ago
Incorrectly formatted remotely hosted links causes new tab to be empty
Categories
(Firefox :: New Tab Page, defect)
Firefox
New Tab Page
Tracking
()
People
(Reporter: Mardak, Assigned: Mardak)
References
Details
Attachments
(2 files)
3.11 KB,
patch
|
adw
:
review+
|
Details | Diff | Splinter Review |
3.23 KB,
patch
|
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
My new tab is showing all tiles with empty tiles. $ curl -L -d '{"locale":"en-US"}' https://tiles.up.mozillalabs.com/v2/links/fetch {"en-US": {"en-US": [{"directoryId": 1, .. There's an extra layer of { "en-US", so when the client accesses "en-US" and expects an array but gets an object, .map fails: http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/DirectoryLinksProvider.jsm#382 382 aCallback(rawLinks.map((link, position) => { rawLinks.map fails and the callback is never called, so newtab page is borked.
Comment 1•9 years ago
|
||
It was an error on my part. I was using a WIP tool for generating the new tile's ID. A correct payload has been uploaded and deployed to the CDN, so the problem should be fixed now.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 2•9 years ago
|
||
For the record, the hosted file has been updated to correctly only have one level of "en-US", so there's only one object with an array inside. Instead of an object with an object with an array. This file was available for ~3 hours, so users who happened to get that file will see empty new tab tiles for 24 hours before the client fetches the correct file.
Updated•9 years ago
|
Resolution: FIXED → INVALID
Assignee | ||
Comment 3•9 years ago
|
||
Reopening: The client should probably be more resilient to invalid data file -- at least not break the rest of new tab UI/functionality. I don't think we'll need to uplift this assuming we're "good" going forwards in terms of serving valid payloads.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Assignee | ||
Comment 5•9 years ago
|
||
For those who are running into this bug, you can go to about:config and change the value for the preference "browser.newtabpage.directory.source" to "", then reset the preference (right-click -> reset).
Assignee | ||
Comment 6•9 years ago
|
||
Sorry, comment 5 isn't enough. After resetting the pref, restart the browser.
Assignee | ||
Comment 8•9 years ago
|
||
Some notes on what's going on: NewTabUtils.populateCache triggers getLinks for each provider and decrements a counter until all providers return. In the meantime, future populateCache calls (happens for each new tab page) get queued because NewTabUtils remembers that it has asked the providers for data. But DirectoryLinksProvider.getLinks never calls its callback (which decrements the counter) because remotely fetched data has an {object} instead of [array], so .map throws an exception. Setting the directory.source pref forces a download of the correct remotely hosted file, but a restart is required because NewTabUtils will wait forever for its counter to decrement to 0.
Updated•9 years ago
|
Status: REOPENED → NEW
Assignee | ||
Comment 9•9 years ago
|
||
[Tracking Requested - why for this release]: Now that we're using live links data with bug 1042876, it would be safer for users' Firefox to not have the possibility of being broken for at least 24 hours if somehow our servers returned the wrong data.
Updated•9 years ago
|
Flags: qe-verify?
Assignee | ||
Comment 10•9 years ago
|
||
Attachment #8494083 -
Flags: review?(adw)
Comment 11•9 years ago
|
||
Tracking (cf comment #9)
status-firefox33:
--- → affected
status-firefox34:
--- → affected
status-firefox35:
--- → affected
tracking-firefox34:
--- → +
Comment 12•9 years ago
|
||
It would be nice if a fix could land soon.
Comment 13•9 years ago
|
||
Comment on attachment 8494083 [details] [diff] [review] v1 Review of attachment 8494083 [details] [diff] [review]: ----------------------------------------------------------------- Great, thanks. As follow-up work, maybe getLinks should return a promise instead of taking a callback to make it more likely errors will be caught. Links could then handle error recovery for all types of providers.
Attachment #8494083 -
Flags: review?(adw) → review+
Assignee | ||
Comment 14•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/dea5ec3d6ef9
![]() |
||
Comment 15•9 years ago
|
||
Should this actually be put on the 35.2 iteration?
Flags: qe-verify? → qe-verify+
QA Contact: cornel.ionce
Comment 16•9 years ago
|
||
When it is resolved it will be. (In reply to Robert Kaiser (:kairo@mozilla.com) from comment #15) > Should this actually be put on the 35.2 iteration?
Comment 17•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/dea5ec3d6ef9
Status: NEW → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
Updated•9 years ago
|
Iteration: --- → 35.2
Comment 18•9 years ago
|
||
Ed, could you ask for an uplift to aurora and beta? Thanks
Flags: needinfo?(edilee)
Assignee | ||
Comment 19•9 years ago
|
||
Flags: needinfo?(edilee)
Assignee | ||
Comment 20•9 years ago
|
||
Comment on attachment 8496360 [details] [diff] [review] for uplift Approval Request Comment [Feature/regressing bug #]: Bug 1030832 / Enhanced Tiles [User impact if declined]: Potential for new tab page to be empty and broken for 24 hours if the server provides a wrong data file [Describe test coverage new/current, TBPL]: Test added for this bug [Risks and why]: Low - minor code changes adding exception handling [String/UUID change made/needed]: None
Attachment #8496360 -
Flags: approval-mozilla-beta?
Attachment #8496360 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
Updated•9 years ago
|
Attachment #8496360 -
Flags: approval-mozilla-beta?
Attachment #8496360 -
Flags: approval-mozilla-beta+
Attachment #8496360 -
Flags: approval-mozilla-aurora?
Attachment #8496360 -
Flags: approval-mozilla-aurora+
Updated•9 years ago
|
Flags: in-testsuite+
Assignee | ||
Comment 21•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/cf719ba91118 status-firefox33 wontfix per bug 1073823
Assignee | ||
Comment 22•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/d34488e06177
Comment 23•9 years ago
|
||
Verified this issue on Windows 7 64-bit, Ubuntu 12.04 and Mac OS X 10.9 using: - latest Nightly, build ID: 20140928030206 - latest Aurora, build ID: 20140928004003
Comment 24•9 years ago
|
||
Also verified on Firefox 33 beta 8, build ID: 20140929180120
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•