Closed
Bug 1396104
Opened 7 years ago
Closed 7 years ago
Use rich icons when getting top sites
Categories
(Firefox :: New Tab Page, enhancement)
Firefox
New Tab Page
Tracking
()
RESOLVED
FIXED
Firefox 57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: Mardak, Assigned: ursula)
References
Details
Attachments
(1 file)
Ursula says 1 line change: This is done via newtabutils's _addFavicons function - passing in a 0 to here: https://dxr.mozilla.org/mozilla-central/source/toolkit/modules/NewTabUtils.jsm?q=file%3Anewtabutils.jsm&redirect_type=single#877 to pull out largest favicon available
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → usarracini
Comment hidden (mozreview-request) |
Reporter | ||
Comment 2•7 years ago
|
||
mozreview-review |
Comment on attachment 8906070 [details] Bug 1396104 - Use rich icons when getting top sites https://reviewboard.mozilla.org/r/177808/#review182876 ::: toolkit/modules/NewTabUtils.jsm:961 (Diff revision 1) > link.faviconLength = len; > + link.faviconSize = size; > } > return resolve(link); > }); > - }).catch(() => { > + }, 0).catch(() => { The indentation is all funky here. Your 0 here is the 2nd argument to `new Promise()` (notice the `.catch`) Does this mean we don't actually need to pass in 0 to `getFaviconDataForPage`?
Reporter | ||
Comment 3•7 years ago
|
||
For styling, I could see the 0 on its own line with a comment, e.g., new Promise(resolve => { return Places.getFavicon( uriStuff, callbackStuff => { }, 0); // preferredWidth: get the biggest width available }).catch Where you only touch the line of the callback closing brace (add comma) and the 0 line.
Comment hidden (mozreview-request) |
Reporter | ||
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8906070 [details] Bug 1396104 - Use rich icons when getting top sites https://reviewboard.mozilla.org/r/177808/#review182888
Attachment #8906070 -
Flags: review?(edilee) → review+
Pushed by edilee@gmail.com: https://hg.mozilla.org/integration/autoland/rev/d923714255be Use rich icons when getting top sites r=Mardak
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d923714255be
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Updated•5 years ago
|
Component: Activity Streams: Newtab → New Tab Page
You need to log in
before you can comment on or make changes to this bug.
Description
•