Closed
Bug 990713
Opened 10 years ago
Closed 10 years ago
Update directoryLinks to have actual links and images
Categories
(Firefox :: Tabbed Browser, defect)
Firefox
Tabbed Browser
Tracking
()
VERIFIED
FIXED
Firefox 32
People
(Reporter: Mardak, Assigned: emtwo)
References
Details
(Whiteboard: p=13 s=it-32c-31a-30b.1 [qa-][talos_regression])
Attachments
(12 files, 7 obsolete files)
3.98 KB,
image/png
|
Details | |
2.24 KB,
image/png
|
Details | |
7.96 KB,
image/png
|
Details | |
5.69 KB,
image/png
|
Details | |
304.61 KB,
patch
|
adw
:
review+
|
Details | Diff | Splinter Review |
156.91 KB,
image/png
|
Details | |
13.78 KB,
image/png
|
Details | |
152.36 KB,
image/png
|
Details | |
2.47 KB,
image/png
|
Details | |
2.31 KB,
image/png
|
Details | |
154.63 KB,
patch
|
Details | Diff | Splinter Review | |
152.05 KB,
image/png
|
Details |
Bug 975475 will be providing the images/colors for the initial list. Bug 975228 landed by avoiding leak failures when we actually had links to sites. The Linux/64 Debug bc1 test leaks include SpdySession31, so somehow one of the spdy-servers in the list (one/some/all of facebook.com, youtube.com, twitter.com see bug 975228 comment 38) is being loaded? favicon? prefetch? The details of the leak from bug 975228 comment 32: https://tbpl.mozilla.org/?tree=Try&rev=5da828c50df4 TEST-UNEXPECTED-FAIL | leakcheck | 4779 bytes leaked (EventTokenBucket, Mutex, ReentrantMonitor, Service, SpdySession3, ...) leaked 2 EventTokenBucket (216 bytes) leaked 10 Mutex (120 bytes) leaked 3 ReentrantMonitor (48 bytes) leaked 1 Service (64 bytes) leaked 1 SpdySession31 (588 bytes) leaked 3 StringAdopt (3 bytes) leaked 1 ThirdPartyUtil (16 bytes) leaked 1 TransportSecurityInfo (116 bytes) leaked 1 nsCategoryObserver (60 bytes) leaked 1 nsCookiePermission (36 bytes) leaked 1 nsCookieService (84 bytes) leaked 1 nsDBusService (24 bytes) leaked 1 nsDNSRecord (28 bytes) leaked 1 nsDNSService (88 bytes) leaked 5 nsDeque (260 bytes) leaked 1 nsEffectiveTLDService (52 bytes) leaked 1 nsHashtable (44 bytes) leaked 1 nsHostRecord (72 bytes) leaked 2 nsHttpAuthCache::AppDataClearObserver (32 bytes) leaked 1 nsHttpConnection (208 bytes) leaked 1 nsHttpConnectionInfo (44 bytes) leaked 1 nsHttpConnectionMgr (160 bytes) leaked 1 nsHttpConnectionMgr::nsConnectionHandle (16 bytes) leaked 1 nsHttpHandler (504 bytes) leaked 1 nsIDNService (72 bytes) leaked 1 nsIOService (104 bytes) leaked 1 nsInterfaceRequestorAgg (24 bytes) leaked 6 nsMainThreadPtrHolder<T> (72 bytes) leaked 1 nsNSSCertificate (56 bytes) leaked 1 nsNSSSocketInfo (196 bytes) leaked 1 nsNetworkManagerListener (24 bytes) leaked 1 nsObserverService (52 bytes) leaked 1 nsPermissionManager (100 bytes) leaked 1 nsPrefBranch (76 bytes) leaked 1 nsSSLStatus (52 bytes) leaked 1 nsSiteSecurityService (60 bytes) leaked 1 nsSocketTransport (332 bytes) leaked 1 nsSocketTransportService (148 bytes) leaked 1 nsStreamConverterService (72 bytes) leaked 18 nsStringBuffer (144 bytes) leaked 7 nsTArray_base (28 bytes) leaked 1 nsThread (124 bytes) leaked 1 nsTimerImpl (68 bytes) leaked 1 nsUnicodeNormalizer (12 bytes) leaked 5 nsWeakReference (80 bytes)
Flags: firefox-backlog?
Reporter | ||
Comment 1•10 years ago
|
||
Anyone have insights into how to debug this Linux/64 Debug bc1 leak failure? We believe it's related to a SPDY site, and somehow the leaktest doesn't fail when I converted all the directoryLinks to https://www.mozilla.org/
Reporter | ||
Comment 2•10 years ago
|
||
The simple fix is to sidestep the issue by Services.prefs.setCharPref(PREF_NEWTAB_DIRECTORYSOURCE, "data:application/json,{}"); at the beginning of mochitest, so all tests that happen to open about:newtab will continue to get blank tiles.
Updated•10 years ago
|
Flags: firefox-backlog? → firefox-backlog+
Reporter | ||
Updated•10 years ago
|
Assignee: nobody → mzhilyaev
Whiteboard: p=13
Updated•10 years ago
|
Status: NEW → ASSIGNED
Whiteboard: p=13 → p=13 s=it-31c-30a-29b.2
Updated•10 years ago
|
Whiteboard: p=13 s=it-31c-30a-29b.2 → p=13 s=it-31c-30a-29b.2 [qa-]
Reporter | ||
Comment 3•10 years ago
|
||
I pushed various tryserver tests that set the url to the actual site instead of mozilla.org at various positions. Facebook #1: Leak https://tbpl.mozilla.org/?tree=Try&rev=af2ca7c08573 YouTube #1: Leak https://tbpl.mozilla.org/?tree=Try&rev=dfee822f5579 Twitter #1: NOleak https://tbpl.mozilla.org/?tree=Try&rev=f806559e46a2 YouTube #3: NOleak https://tbpl.mozilla.org/?tree=Try&rev=efa1bd512cd1 Twitter #6: NOleak https://tbpl.mozilla.org/?tree=Try&rev=76e617d90463 Facebook #9: NOleak https://tbpl.mozilla.org/?tree=Try&rev=1a1bd60dfbfd All 3 of Facebook, YouTube, and Twitter have X-Firefox-Spdy: "3.1" in the response headers, so not sure why Twitter isn't causing leaks when in position #1. I would guess that the mouse pointer is over position 1 and causes some loading? Another note is that the leak is intermittent.
Reporter | ||
Comment 4•10 years ago
|
||
(In reply to Ed Lee :Mardak from comment #3) > Twitter #1: NOleak https://tbpl.mozilla.org/?tree=Try&rev=f806559e46a2 > not sure why Twitter isn't causing leaks when in position #1 Turns out Twitter in spot 1 is leaking. Just at a much lower rate: Twitter 1 leak failure out of 14 Facebook 3 leaks in 4 YouTube 3 leaks in 4 So at least it's consistent-ish.
Reporter | ||
Comment 5•10 years ago
|
||
Putting http://www.facebook.com/ (not https) in spot #1 does not cause the leak. https://tbpl.mozilla.org/?tree=Try&rev=544389210a65
Comment 6•10 years ago
|
||
Could y'all annotate the strings on the placeholder tiles on about:newtab to something like BBC(placeholder) ? It's very confusing to click on a BBC tile and get https://www.mozilla.org/en-US/#http://www.bbc.co.uk/ instead. It looks like broken behavior instead of 'hang on, we're working on this'
Reporter | ||
Comment 7•10 years ago
|
||
(In reply to :Ally Naaktgeboren from comment #6) > Could y'all annotate the strings on the placeholder tiles on about:newtab to > something like BBC(placeholder) ? But but... there's a big PLACEHOLDER image above the string. ;) This would get fixed through bug 991853 where we'll put in actual links and images to Mozilla properties for now.
Comment 8•10 years ago
|
||
(In reply to Ed Lee :Mardak from comment #7) > (In reply to :Ally Naaktgeboren from comment #6) > > Could y'all annotate the strings on the placeholder tiles on about:newtab to > > something like BBC(placeholder) ? > But but... there's a big PLACEHOLDER image above the string. ;) This would > get fixed through bug 991853 where we'll put in actual links and images to > Mozilla properties for now. Right, so it's clear that image temporary, not that the link is borked. :)
Comment 9•10 years ago
|
||
This is from the speculative connections being made[1] from tiles on the new tab page. See bug 964358 and bug 964369 comment 3 (and the patch there) where we may disable these connections by default in tests. [1] https://mxr.mozilla.org/mozilla-central/source/browser/base/content/newtab/sites.js?rev=f3c44cfc3900#182
Reporter | ||
Comment 10•10 years ago
|
||
Ah ha! I should have looked deeper into the newtab code. I was grepping for prefetch or preload. :( Here are some notes from maksik: I am running tests 24x7 to repro the leak with minimal luck so far: 1) I did manage to compile debug+refcount version of fx on linux 64bit 2) running it is a major pain, as it often either hangs or exit with cascading errors: "JavaScript Error: "source.defaultView is null". 3) sometimes the VM instance gets horcked and i have to reboot it 4) I did manage to complete a run of mochitest-browser for browser/base and broswer/components which cover 99.9% of all the tests in BC1 bucket that triggers the leak. AND.... I saw no leaks. The item 4) above is alerting. I am using our github repo master branch, not the fxteam branch. Which bags the question if our changes are implicated to begin with. Given how painful the bug is, I am going to schedule a meeting to chat of how we could be systematic about reproducing the leak. We may still have to look for a real linux box, as VM on my poor machine does not seem to be happy running these massive tests.
Reporter | ||
Comment 11•10 years ago
|
||
Bug 992611 turns off speculative connects for tests and this should prevent the leak failures.
Depends on: 992611
Updated•10 years ago
|
Whiteboard: p=13 s=it-31c-30a-29b.2 [qa-] → p=13 [qa-]
Comment 13•10 years ago
|
||
Boriss, can you update the YouTube logo and color according to these links? https://developers.google.com/youtube/branding_guidelines https://www.youtube.com/yt/brand/using-logo.html
Flags: needinfo?(jboriss)
Comment 14•10 years ago
|
||
Borris, here's our ready to go list so far: +–––––––––––––––––––––––––+ | | | +–––––+ +–––––+ +–––––+ | | | 1 | | 2 | | 3 | | | +–––––+ +–––––+ +–––––+ | | | | +–––––+ +–––––+ +–––––+ | | | 4 | | 5 | | 6 | | | +–––––+ +–––––+ +–––––+ | | | | +–––––+ +–––––+ +–––––+ | | | 7 | | 8 | | 9 | | | +–––––+ +–––––+ +–––––+ | | | +–––––––––––––––––––––––––+ Directory Tiles with background colors: 1. 2. youtube.com, #e5523f 3. twitter.com, #00b5f0 4. wikipedia.com, #ffffff 5. 6. 7. wired.com, #1facdf 8. 9. Ready Alternatives (throw these in somewhere): * mozilla.org, #4d4e54 * amazon.com, #000000 * trulia.com Not (Yet) Ready Alternates: * facebook.com, #3a5898 * reddit.com, * weather.com, Not Ready Alternates: 10. lonelyplanet.com, #002f74 11. wordpress.com, #008fc0 12. nytimes.com, #0093b9 13. eff.org, #000000 14. npr.org, #c7d2d7
Comment 15•10 years ago
|
||
(In reply to Bryan Clark (Firefox Search PM) [:clarkbw] from comment #13) > Boriss, can you update the YouTube logo and color according to these links? > > https://developers.google.com/youtube/branding_guidelines > https://www.youtube.com/yt/brand/using-logo.html Attached is a white logo, background is #424242
Flags: needinfo?(jboriss)
Comment 16•10 years ago
|
||
Comment 17•10 years ago
|
||
Reporter | ||
Comment 18•10 years ago
|
||
Boriss, these logo files you've provided aren't of the tile size. How should we make them fit the tile? Just take the logo file and have it centered in the tile space and fill in with background color?
Reporter | ||
Comment 19•10 years ago
|
||
Boriss, nevermind. These show up fine when centered/centered. Originally I was confused because attachment 8392628 [details] says 112x82 or smaller and these logo files are 180x86
Comment 20•10 years ago
|
||
Amazon wanted a version of the logo without the ".com" - new version attached
Comment 21•10 years ago
|
||
(In reply to Ed Lee :Mardak from comment #19) > Boriss, nevermind. These show up fine when centered/centered. Originally I > was confused because attachment 8392628 [details] says 112x82 or smaller and > these logo files are 180x86 Np. It's possible you'll need different version for retina, I imagine. I included the original images but can make 2x size if needed.
Reporter | ||
Updated•10 years ago
|
Assignee: mzhilyaev → msamuel
Whiteboard: p=13 [qa-] → p=13 s=it-31c-30a-29b.3 [qa-]
Reporter | ||
Comment 22•10 years ago
|
||
clarkbw, is this ordering and type pairing correct? 1. amazon organic 2. youtube organic 3. wired sponsored 4. wikipedia organic 5. mozilla affiliate 6. twitter organic 7. lightbeam affiliate 8. trulia sponsored 9. ? eff affiliate ? facebook organic ?
Assignee | ||
Comment 23•10 years ago
|
||
Assignee | ||
Comment 24•10 years ago
|
||
Reporter | ||
Comment 25•10 years ago
|
||
(In reply to Marina Samuel [:emtwo] from comment #23) > Created attachment 8412131 [details] [diff] [review] > WIP: Applying the assets available so far Thanks, you can find more assets in bug 995001
Assignee | ||
Comment 26•10 years ago
|
||
Attachment #8412131 -
Attachment is obsolete: true
Assignee | ||
Comment 27•10 years ago
|
||
Attachment #8412133 -
Attachment is obsolete: true
Reporter | ||
Comment 28•10 years ago
|
||
(In reply to Jennifer Morrow [:Boriss] (Firefox UX) from comment #16) > Created attachment 8407856 [details] > Image: Trulia white logo without wordmark (preferred), background #9acb3c Where did you get that color for Trulia? I see another document with #9ACA3C but their website seems to have a darker green #5EAB1F
Reporter | ||
Comment 29•10 years ago
|
||
There might be some image/url/title/type tweaks, but the code changes should be good.
Attachment #8412161 -
Attachment is obsolete: true
Attachment #8412238 -
Flags: review?(adw)
Reporter | ||
Comment 30•10 years ago
|
||
Attachment #8412162 -
Attachment is obsolete: true
Reporter | ||
Comment 31•10 years ago
|
||
Is this okay Boriss? I took the lightbeam wordmark from the page and resized to 180 wide then added transparency to 86px tall.. (although technically the extra transparency above/below isn't necessary).
Attachment #8412240 -
Flags: ui-review?(jboriss)
Comment 32•10 years ago
|
||
Comment on attachment 8412238 [details] [diff] [review] v1 Review of attachment 8412238 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/content/directoryLinks.json @@ +1,4 @@ > { > "en-US": [ > { > + "url": "http://www.amazon.com", Nit: the other URLs have trailing slashes (which is more "correct" anyway).
Attachment #8412238 -
Flags: review?(adw) → review+
Updated•10 years ago
|
Whiteboard: p=13 s=it-31c-30a-29b.3 [qa-] → p=13 s=it-32c-31a-30b.1 [qa-]
Reporter | ||
Comment 33•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/79989d4a8d24 leave-open for potentially more image/position/title/type/url changes.
Keywords: leave-open
Comment 35•10 years ago
|
||
Removing from iteration until final decisions are made regarding potentially more image/position/title/type/url changes.
Status: ASSIGNED → NEW
Whiteboard: p=13 s=it-32c-31a-30b.1 [qa-] → p=13 [qa-]
Comment 36•10 years ago
|
||
Comment 37•10 years ago
|
||
Comment 38•10 years ago
|
||
Here's the "final" order/type/text/url = Order = 1. Facebook 2. YouTube 3. Wired 4. Lightbeam 5. MoFo 6. Wikipedia 7. Trulia 8. Amazon 9. Webmaker = Types = - Organic - Facebook YouTube Wikipedia Amazon - Affiliate - Lightbeam MoFo Webmaker - (Trial) Sponsored - Wired Trulia = Text:URL = Facebook : https://www.facebook.com/ YouTube : http://www.youtube.com/ Wired : http://www.wired.com/ Lightbeam: https://www.mozilla.org/en-US/lightbeam/ MoFo : http://www.mozilla.org/en-US/about/ Wikipedia: http://en.wikipedia.org/wiki/Main_Page Trulia : http://www.trulia.com/?ecampaign=tiles Amazon : http://www.amazon.com/gp/bit/amazonbookmark.html?tag=mozilla-directory-tiles-20&partner=Mozilla Webmaker : https://webmaker.org/ Only reason for the "final" in quotes is the Wired creative and the Mozilla center tile are being reviewed right now. Everything else should be good.
Reporter | ||
Comment 39•10 years ago
|
||
Attachment #8414783 -
Flags: review?(adw)
Reporter | ||
Comment 40•10 years ago
|
||
Reporter | ||
Comment 41•10 years ago
|
||
Attachment #8414783 -
Attachment is obsolete: true
Attachment #8414783 -
Flags: review?(adw)
Attachment #8414785 -
Flags: review?(adw)
Updated•10 years ago
|
Attachment #8414785 -
Flags: review?(adw) → review+
Comment 42•10 years ago
|
||
this was backed out: https://hg.mozilla.org/integration/fx-team/rev/24cf989be9e3 tracked in bug 1000459. To add more to this, we have a performance regression from this patch, 5.33% TART regression on win7 and winXP. When we backed it out, the regression went away: http://graphs.mozilla.org/graph.html#tests=[[293,132,25]]&sel=none&displayrange=7&datatype=running I had done some retriggers to verify it was a real regression and not a random spike of electricity or some noise: https://tbpl.mozilla.org/?tree=Fx-Team&startdate=2014-04-29&enddate=2014-04-31&jobname=Windows%207%2032-bit%20fx-team%20talos%20svgr
Comment 43•10 years ago
|
||
oh and this also had a 2.22% regression on linux32
Updated•10 years ago
|
Whiteboard: p=13 [qa-] → p=13 [qa-][talos_regression]
Reporter | ||
Comment 44•10 years ago
|
||
jmaher, the regression is the same as bug 990212 where the change that actually triggers the regression is the one line change to turn on the feature in bug 1000459. I believe the plan is to populate the tiles for TART to have the test run consistently (no matter what we change the list of links) and be more "realistic" where users will be seeing tiles with content as opposed to all empty.
Comment 45•10 years ago
|
||
ah, thanks! I had blinders on and just filed things, thanks for clarifying the relationship!
Comment 46•10 years ago
|
||
Proposed background: #1D4F8D Disclaimer: I do some work for LonelyPlanet and I asked their designers to prepare a logo.
Reporter | ||
Comment 47•10 years ago
|
||
Updated various ordering, inserted bbc to spot 9 (moving webmaker to 10), Wired -> WIRED, new WIRED logo, Mozilla Foundation -> The Nonprofit Behind Firefox, updated urls, crushed images
Attachment #8414785 -
Attachment is obsolete: true
Reporter | ||
Comment 48•10 years ago
|
||
Attachment #8414784 -
Attachment is obsolete: true
Comment 49•10 years ago
|
||
(In reply to Piotr Okoński from comment #46) > Created attachment 8417963 [details] > LonelyPlanet logo > > Proposed background: #1D4F8D > > Disclaimer: I do some work for LonelyPlanet and I asked their designers to > prepare a logo. Thanks Piotr, this is great! I sent a message to someone who has our contact at Lonely Planet so we can keep in touch with them.
Reporter | ||
Comment 50•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/4d19381c20ee
Status: NEW → ASSIGNED
Keywords: leave-open
Whiteboard: p=13 [qa-][talos_regression] → p=13 s=it-32c-31a-30b.1 [qa-][talos_regression]
Comment 51•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4d19381c20ee
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 32
Updated•10 years ago
|
Status: RESOLVED → VERIFIED
Reporter | ||
Updated•10 years ago
|
Attachment #8412240 -
Flags: ui-review?(jboriss)
Comment hidden (spam) |
You need to log in
before you can comment on or make changes to this bug.
Description
•