Closed Bug 598174 Opened 9 years ago Closed 9 years ago

Frontend loads the favicon unconditionally

Categories

(Firefox for Android Graveyard :: General, defect)

x86
Linux
defect
Not set

Tracking

(fennec2.0b2+)

VERIFIED FIXED
Tracking Status
fennec 2.0b2+ ---

People

(Reporter: jdm, Assigned: wesj)

References

Details

Attachments

(1 file, 3 obsolete files)

Whereas Firefox has code like http://mxr.mozilla.org/mozilla-central/source/browser/base/content/tabbrowser.xml#634, Fennec goes ahead and loads anything it can.  This ends up bringing up an error message on FTP sites, which is bad news bears.
Sounds like good cleanup. Should be a simple fix.
tracking-fennec: --- → 2.0b2+
Assignee: nobody → wjohnston
Attached patch Fix... (obsolete) — Splinter Review
This replicates most of what Firefox does. I removed the prefs, as they seem like a waste of time. I also added "about" protocols, because we use favicons for about:home and the like.
Attachment #477719 - Flags: review?(mark.finkle)
Attached patch Ack! Fix without cruft (obsolete) — Splinter Review
Forgot to hit save...
Attachment #477719 - Attachment is obsolete: true
Attachment #477719 - Flags: review?(mark.finkle)
Comment on attachment 477720 [details] [diff] [review]
Ack! Fix without cruft

I assumed you wanted this reviewed too
Attachment #477720 - Flags: review+
Whiteboard: [fennec-checkin-postb1]
Attached patch unbitrotted for checkin (obsolete) — Splinter Review
Unbitrotted for checkin, but it causes a test failure"

TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/mobile/chrome/browser_navigation.js | The page favicon must be loaded - Got , expected data:image/gif;base64,R0lGODlhCwALAIAAAAAA3pn/ZiH5BAEAAAEALAAAAAALAAsAAAIUhA+hkcuO4lmNVindo7qyrIXiGBYAOw==
Attachment #479593 - Attachment is patch: true
Attachment #479593 - Attachment mime type: application/octet-stream → text/plain
Attached patch Fix errorsSplinter Review
This passes tests for me. Mochitests expect to set icons on pages from chrome protocols, which we didn't allow. So now we allow them. I'll follow up with a test for a protocol we don't allow... as soon as I figure out what that is (ftp isn't on right now... moz-icon?)
Attachment #477720 - Attachment is obsolete: true
Attachment #479593 - Attachment is obsolete: true
Attachment #479606 - Flags: review+
pushed:
http://hg.mozilla.org/mobile-browser/rev/b71c4fd32aa4
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [fennec-checkin-postb1]
Verified fixed on:
Mozilla/5.0 (Android;Linux armv7l;rv:9.0a1)Gecko/20110913
Firefox/9.0a1 Fennec/9.0a1
Device: Samsung Galaxy S
OS: Android 2.2
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.