If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Not displaying icons for photos.google.com

VERIFIED FIXED in Firefox OS master

Status

Firefox OS
Gaia::System::Browser Chrome
P2
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: benfrancis, Assigned: sfoster)

Tracking

unspecified
FxOS-S10 (30Oct)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(blocking-b2g:2.5+, b2g-master fixed)

Details

(Whiteboard: [systemsfe])

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
STR:
* Navigate to photos.google.com (may need a Google account to log in)
* Observe icon in browser chrome

Expected:
* Displays icon from https://ssl.gstatic.com/social/photosui/images/logo_photos_color_192.png

Actual:
* Displays default icon

Works in desktop Firefox.

I suspect the issue may be that the provided href attribute of the icon link relation is "//ssl.gstatic.com/social/photosui/images/logo_photos_color_192.png", without "http:"/"https:" at the start, i.e. it is a scheme-relative URL.
blocking-b2g: --- → 2.5+
Priority: -- → P2
Duplicate of this bug: 1203136
See for https://bugzilla.mozilla.org/show_bug.cgi?id=1203136#c2 for more info.

Updated

2 years ago
QA Whiteboard: [COM=Pin the Web]

Comment 3

2 years ago
Created attachment 8676421 [details] [review]
[gaia] sfoster:missing-icon-bug-1188026 > mozilla-b2g:master
(Assignee)

Comment 4

2 years ago
This doesnt seem to have anything to do with the scheme-relative urls - so I changed the bug title. When I logged out loading photos.google.com, I see the mozbrowsericonchange that gives us the correct icon. But after that I see 2 more mozbrowserlocationchange events which *dont actually represent a change of location*. Regardless, AppWindow wipes out the favicons collection so by the time loadend comes around we have nothing to display.
(Assignee)

Comment 5

2 years ago
Comment on attachment 8676421 [details] [review]
[gaia] sfoster:missing-icon-bug-1188026 > mozilla-b2g:master

As a first step, this adds an equality check in AppWindow's mozbrowserlocationchange handler before resetting this.favicons. But this is probably not enough. AppChrome is making bad assumptions about the state of the page and what icons it has collected. if AppWindow is the source of truth on this stuff, maybe we should move the origin comparison at https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_chrome.js#L1024? 

Also, wtf is with these bogus mozbrowserlocationchange events?
Attachment #8676421 - Flags: feedback?(apastor)
(Assignee)

Updated

2 years ago
Assignee: nobody → sfoster
Summary: Not displaying icons for icon link relations with scheme-relative URL → Not displaying icons for photos.google.com
(Assignee)

Comment 6

2 years ago
Comment on attachment 8676421 [details] [review]
[gaia] sfoster:missing-icon-bug-1188026 > mozilla-b2g:master

First time I tested this early return I was getting the neterror/untrusted connection trying to connect to photos.google.com, but I've reset and re-applied the patch and I've not seen it since. I'm watching the tests carefully. AFAICT a mozbrowserlocationchange event that doesnt represent a change in location should be a no-op in AppWindow. I've examined the events and there's no other differences I can spot, so the early return seems safe and reasonable.
Attachment #8676421 - Flags: feedback?(apastor) → review?(apastor)
The code looks good, but gaia-try doesn't seem very happy. I'll check the tests later
(Assignee)

Comment 8

2 years ago
Comment on attachment 8676421 [details] [review]
[gaia] sfoster:missing-icon-bug-1188026 > mozilla-b2g:master

Cancelling review while I look into those tests.
Attachment #8676421 - Flags: review?(apastor)
(Assignee)

Comment 9

2 years ago
Comment on attachment 8676421 [details] [review]
[gaia] sfoster:missing-icon-bug-1188026 > mozilla-b2g:master

Tests look much happier, re-flagging for review.
Attachment #8676421 - Flags: review?(apastor)
Comment on attachment 8676421 [details] [review]
[gaia] sfoster:missing-icon-bug-1188026 > mozilla-b2g:master

Left a question in GH, but r=me overall. Thanks!
Attachment #8676421 - Flags: review?(apastor) → review+
(Assignee)

Comment 11

2 years ago
(In reply to Alberto Pastor [:albertopq] from comment #10)
> Comment on attachment 8676421 [details] [review]
> [gaia] sfoster:missing-icon-bug-1188026 > mozilla-b2g:master
> 
> Left a question in GH, but r=me overall. Thanks!

I moved the title update inside the if(). 

Merged to master: https://github.com/mozilla-b2g/gaia/commit/9bd77ab486742eef503bb252d215746a2d0b1638
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-b2g-master: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S10 (30Oct)

Comment 12

2 years ago
Verified on

[Flame]
Build ID               20151027150240
Gaia Revision          a26eadc5e1133d5112b6cbc10badbb7670a1090f
Gaia Date              2015-10-27 17:36:52
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/2b333a1d94e805a59c619ee41a6dec7fdcce505d
Gecko Version          44.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20151027.184038
Firmware Date          Tue Oct 27 18:40:49 EDT 2015
Bootloader             L1TC000118D0

[Aries]
Build ID               20151027221526
Gaia Revision          a26eadc5e1133d5112b6cbc10badbb7670a1090f
Gaia Date              2015-10-27 17:36:52
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/2b333a1d94e805a59c619ee41a6dec7fdcce505d
Gecko Version          44.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20151027.213419
Firmware Date          Tue Oct 27 21:34:27 UTC 2015
Bootloader             s1
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.