Closed Bug 598174 Opened 14 years ago Closed 14 years ago

Frontend loads the favicon unconditionally

Categories

(Firefox for Android Graveyard :: General, defect)

Product:
Firefox for Android Graveyard
Component:
General
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(fennec2.0b2+)

Status:
VERIFIED FIXED
Tracking Flags:
Tracking Status
fennec 2.0b2+ ---

People

(Reporter: jdm, Assigned: wesj)

References

Details

Attachments

(1 file, 3 obsolete files)

Fix errors
3.60 KB, patch
wesj
: review+
Details | Diff | Splinter Review 
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 errors — — Splinter 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: 14 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.

Attachment

General

Creator:
Created:
Updated:
Size: