Closed Bug 571812 Opened 10 years ago Closed 10 years ago

Remove setIcon

Categories

(Firefox for Android Graveyard :: General, defect)

x86
Linux
defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: mfinkle, Assigned: mfinkle)

Details

Attachments

(2 files)

Since mIconURL support has been added to browser XBL, we can remove setIcon. I don't see any need for it.

I removed it and tested loading pages without favicons, pages with favicons, closing tabs and switching tabs. Everything works.
Attached patch patchSplinter Review
forgot to add the patch!
Assignee: nobody → mark.finkle
Attachment #450975 - Flags: review?(21)
(In reply to comment #0)
> Since mIconURL support has been added to browser XBL, we can remove setIcon. I
> don't see any need for it.
> 
> I removed it and tested loading pages without favicons, pages with favicons,
> closing tabs and switching tabs. Everything works.

I suspect this to be needed for background loading tabs. I'll apply the patch and see what happen.
Comment on attachment 450975 [details] [diff] [review]
patch

(In reply to comment #2)
> (In reply to comment #0)
> > Since mIconURL support has been added to browser XBL, we can remove setIcon. I
> > don't see any need for it.
> > 
> > I removed it and tested loading pages without favicons, pages with favicons,
> > closing tabs and switching tabs. Everything works.
> 
> I suspect this to be needed for background loading tabs. I'll apply the patch
> and see what happen.

It work well for me!
pushed to m-b:
http://hg.mozilla.org/mobile-browser/rev/2935fc0e9bb7
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
We need to backout the patch I think. :(

Steps to reproduce: 
 * start fennec on about:home
 * go to google.com
 * use the back button to return to about:home
 * use the forward button to return to google.com

Actual result:
 * the favicon is missing

Reverting to a revision before this one make it works. Sorry for have not noticed that sooner.
I'll try to fix it first
Attached patch followup fixSplinter Review
This followup patch moves the fallback code out of DOMContentLoaded and into pageshow message. DOMContentLoaded does not fire when pages are loaded from cache.

I removed the message handler for DOMContentLoaded since it is no longer used.
Attachment #451289 - Flags: review?(21)
Verified fixed on:
Mozilla/5.0 (Android;Linux armv7l;rv:9.0a1)Gecko/20110921
Firefox/9.0a1 Fennec/9.0a1
Device: HTC Desire
OS: Android 2.2
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.