Closed Bug 1063685 Opened 10 years ago Closed 10 years ago

[B2G][Browser] Top Sites in Browser app does not display images for recently visited sites

Categories

(Firefox OS Graveyard :: Gaia::System::Browser Chrome, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(b2g-v1.4 wontfix, b2g-v2.0 wontfix, b2g-v2.1 verified, b2g-v2.2 verified)

VERIFIED FIXED
2.1 S5 (26sep)
Tracking Status
b2g-v1.4 --- wontfix
b2g-v2.0 --- wontfix
b2g-v2.1 --- verified
b2g-v2.2 --- verified

People

(Reporter: ddixon, Assigned: daleharvey, NeedInfo)

References

Details

(Whiteboard: [2.1-FL-bug-bash][systemsfe])

Attachments

(3 files, 1 obsolete file)

Description:
After visiting most websites in Browser App, there are no images displayed in under "Top Sites".  There are only blank icons/windows with the name of the web page across the bottom.  


Repro Steps:
1) Update a Flame to 20140904000203
2) Open Browser and go to a website
3) Tap ellipsis>new window


Actual Results: Top Sites does not display an image for the website that was recently visited. 


Expected Results: Top Sites displays images after website is visited. 

Environmental Variables:
Device: Flame 2.1 (512 MB)
Build ID: 20140904000203
Gaia: a47ecb6368c015dd72148acde26413fd90ba3136
Gecko: 757931d0149e
Version: 34.0a2 (Master)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0


Repro frequency: 9/10, 90%

See attached: screenshot, logcat
QA Whiteboard: [QAnalyst-Triage?]
Attached image topsites_no_images.png
Flags: needinfo?(jmitchell)
Whiteboard: [2.1-FL-bug-bash]
Branch Checks

Issue DOES occur in Flame 2.2, 2.1 (319 MB), 2.0, 1.4, and Open_C 2.2 


Device: Flame Master (2.2)
Build ID: 20140905031309
Gaia: 5765c62163bcb7fde5ebfd211881117de31a7c46
Gecko: dddbe46f3ceb
Version: 35.0a1 (Master)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0
---------------------------------------------------------
Device: Flame 2.1 (319 MB) 
Build ID: 20140904000203
Gaia: a47ecb6368c015dd72148acde26413fd90ba3136
Gecko: 757931d0149e
Version: 34.0a2 (Master)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
---------------------------------------------------------
Device: Flame 2.0
Build ID: 20140905063812
Gaia: 4627014cc5c5eeec894183866d4c57291302f8b8
Gecko: 1c8f11212f9c
Version: 32.0 (2.0)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
---------------------------------------------------------
Device: Flame 1.4
Build ID: 20140903033752
Gaia: 2ee5b00bfbb8a67a967094804390b4afce8ecf54
Gecko: f5a75c0dd74e
Version: 30.0 (1.4)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0
---------------------------------------------------------
Device: Open_C Master (2.2)
Build ID: 20140905031309
Gaia: 5765c62163bcb7fde5ebfd211881117de31a7c46
Gecko: dddbe46f3ceb
Version: 35.0a1 (Master)
Firmware Version: P821A10V1.0.0B06_LOG_DL
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0
Component: Gaia::Browser → Gaia::System::Browser Chrome
triage - not a regression - on the fence with the nom - does seem like something we should have
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
There are pathalogical cases where this may not be fixable, but this is broken very commonly and is a regression compared to the previous browser, I will fix and suggest it blocked
Assignee: nobody → dale
I had filed bug 1058943 for this, but since this has a screenshot, let's keep this instead.
2 issues here, one will be fixed by https://bugzilla.mozilla.org/show_bug.cgi?id=1063789 (just pushed to try), this causes app:// to be the most visited sites

The other is that some screenshots are apparently just coming back as being white, debugging now
QA Whiteboard: [QAnalyst-Triage+][lead-review+]
So there are a few different cases here

https://bugzilla.mozilla.org/show_bug.cgi?id=1064920 - filed for http://guardian.co.uk, it loads indefinitely so we dont ever take a screenshot

https://bugzilla.mozilla.org/show_bug.cgi?id=1064925 - filed for bbc.co.uk, some sites redirect immediately and wont ever have a screenshot or icon

This bug I will use specifically for cases like m.bbc.co.uk and google.com who get an initial blank white screenshot, and on second load the proper screenshot is shown
Whiteboard: [2.1-FL-bug-bash] → [2.1-FL-bug-bash][systemsfe]
Target Milestone: --- → 2.1 S5 (26sep)
ok finally found the issue for this, for website redirects the origin stays with the original url while the config.url changes
This issue here is that the origin needs to match for getApp(url) to pick up the browser app, this will only ever match the first loaded website when no redirects happen, origin isnt updated as the user navigates so the test was always wrong.
Attachment #8493334 - Flags: review?(alive)
Comment on attachment 8493334 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/24305

Looks sane, but not sure why this fixes the problem. Could you elaborate more?
Attachment #8493334 - Flags: review?(alive) → review+
Both app launches and browser sessions are opened with a config that contains an origin, the origin never gets updated, with applications getApp is searched with the manifest origin (and possibly manifestUrl) so it is fine for searching App windows, however browser windows are only searched by their current url which is only stored within app.config.url, if the app.config.url is the same as the origin (the first time its opened) it matches which is the use case we usually test with (freshly opened windows) but any other time it wont match.
https://github.com/mozilla-b2g/gaia/commit/a0fa29db8e9e15afe3b1787bf494caa86a033f10

Will ask for uplift on 2.1 once this goes green (just realised I checked the wrong tbpl results and this one didnt actually trigger a gaia try run, sorry)
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Reverted in https://github.com/mozilla-b2g/gaia/commit/f20d360297e82cf5862f7f159c837af4f557c2ea for causing an intergration test failure, my bad for pushing without checking try properly
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
So getApp has existing semantics that I dont quite understand that dont match what we were using it for in places, Ill do a patch that adds |getAppAtURL| if that sounds good
Sorry for screwing that up, I dont think its a good idea to modify getApp to what we want in places as its used elsewhere, this is safer and clearer
Attachment #8493334 - Attachment is obsolete: true
Attachment #8493768 - Flags: review?(alive)
Green run, landed: https://github.com/mozilla-b2g/gaia/commit/e68f68150b89887db48acd59b8a66c3cb7f9a5bc
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Comment on attachment 8493768 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/24342

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): 
[User impact] if declined: a large number of missing screenshots on browser home page
[Testing completed]: unit tests added and manual testing
[Risk to taking this patch] (and alternatives if risky): Its a low risk patch, added a new function that doesnt effect any other functionality
[String changes made]:
Attachment #8493768 - Flags: approval-gaia-v2.1?
Attachment #8493768 - Flags: approval-gaia-v2.1? → approval-gaia-v2.1+
Issue is verified fixed in Flame 2.2, 2.1 (Full Flash, nightly). 

Actual Results: Top Sites display images after they have been visited.  

Note: Comment 4 states this will NOT be fixable in some cases. 

Device: Flame 2.2
BuildID: 20141031061804
Gaia: a07994714f0552f89801d6097982308d8b0a1ee1
Gecko: 6bd2071b373f
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 36.0a1 (2.2)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Device: Flame 2.1
BuildID: 20141031001201
Gaia: f89c7b12c36572262c9ea76058694a139b1a8634
Gecko: 50d48f8a04c7
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 34.0 (2.1)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+][lead-review+] → [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker) → needinfo?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: