Closed
Bug 962025
Opened 11 years ago
Closed 11 years ago
favicon of default bookmarks changes after upgrading firefox
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(fennec-)
RESOLVED
WONTFIX
Tracking | Status | |
---|---|---|
fennec | - | --- |
People
(Reporter: angelc04, Assigned: rnewman)
Details
(Keywords: regression, reproducible)
Attachments
(3 files)
This is a regression bug.
1. Install Firefox V26.0
2. Launch Firefox, you will see favicons for default bookmarks show correct icon.
3. Install firefox v27.0
4. Launch Firefox and check Bookmarks
--> favicons for addons.mozilla.org and support.mozilla.org changed.
upgrading from v26.0 to v26.0.1 has no problem.
Reporter | ||
Updated•11 years ago
|
Keywords: regression
Reporter | ||
Comment 1•11 years ago
|
||
BTW, only default bookmarks has this problem. User added bookmarks shows favicon as expected.
Comment 2•11 years ago
|
||
Screen shots?
Comment 3•11 years ago
|
||
I see no difference whatsoever.
Updated•11 years ago
|
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → WORKSFORME
Reporter | ||
Comment 4•11 years ago
|
||
Aaron, I can consistently reproduce this. Here are the tests I have done.
Release versions:
fennec 25.0 -> fennec 26.0 Pass
fennec 26.0 -> fennec 26.0.1 Pass
fennec 26.0 -> fennec 27.0 (local compiled) Fail
beta versions:
fennec 25.0b8 -> fennec 26.0b10 Pass
fennec 26.0b2 -> fennec 26.0b8 Pass
fennec 26.0b10 -> fennec 27.0b8 Fail
fennec 26.0b10 -> fennec 27.0b1 Fail
fennec 27.0b6 -> fennec 27.0b8 Pass
So the conclusion is:
1. This is a regression bug.
2. It only appears when upgrading from 26.0 to 27.0 build.
3. It cannot be reproduced by upgrading between 27 beta builds.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Reporter | ||
Comment 5•11 years ago
|
||
Reporter | ||
Comment 6•11 years ago
|
||
The device I used.
Samsung GT-P7510, Android 4.0.4
MIUI 2S, android 4.1.1
Updated•11 years ago
|
Keywords: regressionwindow-wanted
Summary: favicon of default bookmarks changes after upgrading firefox → favicon of default bookmarks changes after upgrading firefox (non en-US)
Reporter | ||
Comment 7•11 years ago
|
||
Aaron, why "non en-US" was added to the summary? I dont't think this is related to locale bcz the behavior is the same to en-us locale.
I think this might relate to Favicon Revamp in v27.0 (Bug 914296). But still need to do some test to narrow down the cause.
Flags: needinfo?(aaron.train)
Comment 8•11 years ago
|
||
I haven't been able to reproduce this at all. If you have steps, they would be much appreciated.
Flags: needinfo?(aaron.train)
Keywords: steps-wanted
Reporter | ||
Comment 9•11 years ago
|
||
Sorry for the late reply. I'm currently on holiday.
Here are the steps I used.
0. prepare a clean device
1. install any v26.0 beta build
2. Launch firefox, you will see all bookmark icons shows correctly
3. upgrade firefox to any v27.0 beta build
4. Launch firefox again
--> favicons for addons.mozilla.org and support.mozilla.org changed
Comment 10•11 years ago
|
||
Thanks.
Using the steps in comment #9 with
http://ftp.mozilla.org/pub/mozilla.org/mobile/candidates/26.0b10-candidates/build1/android/multi/fennec-26.0b10.multi.android-arm.apk
Launched, and then upgrading to the first 27 beta candidate
http://ftp.mozilla.org/pub/mozilla.org/mobile/candidates/27.0b1-candidates/build1/android/multi/fennec-27.0b1.multi.android-arm.apk
I was able to reproduce with only an upgraded profile.
On startup with the same dirty profile, I see:
D/LoadFaviconTask(15263): Fetching favicon from JAR.
D/GeckoFavicons(15263): Cancelling favicon load (4)
Perhaps our resident favicon experts might know what happened between 26 and 27 in this timeframe.
tracking-fennec: --- → ?
Flags: needinfo?(rnewman)
Flags: needinfo?(chriskitching)
Summary: favicon of default bookmarks changes after upgrading firefox (non en-US) → favicon of default bookmarks changes after upgrading firefox
Comment 11•11 years ago
|
||
I wonder if this is fallout from bug 941868
Updated•11 years ago
|
Flags: needinfo?
Updated•11 years ago
|
Flags: needinfo?
Assignee | ||
Comment 12•11 years ago
|
||
We touched how some distributed icons were handled (the old way was dumb from an I/O standpoint), but that should only affect about:home.
Icons for built-in bookmarks should still be in the DB from the prior install (filled from the distribution file), and should pass through the cache as normal when subsequently viewed.
Why test against the *first* 27 candidate, not the last?
What happens if you now visit one of those bookmarks?
Flags: needinfo?(rnewman)
Comment 13•11 years ago
|
||
In our latest candidate (26.0b10 to 27.0b10) on an upgraded profile, launch:
D/FaviconCache(19661): Favicon cache fullness: 0/524288
D/GeckoTabs(19661): Setting about: tab favicon inline.
D/GeckoToolbar(19661): setFavicon(android.graphics.Bitmap@43476730)
D/GeckoToolbar(19661): setFavicon(android.graphics.Bitmap@43476730)
D/GeckoToolbar(19661): Ignoring favicon set: new favicon is identical to previous favicon.
D/GeckoToolbar(19661): onTabChanged: FAVICON
D/GeckoToolbar(19661): setFavicon(android.graphics.Bitmap@435c6468)
D/GeckoBrowserApp(19661): BrowserApp.onTabChanged: 0: FAVICON
D/GeckoToolbar(19661): setFavicon(android.graphics.Bitmap@435c6468)
D/GeckoToolbar(19661): Ignoring favicon set: new favicon is identical to previous favicon.
D/GeckoToolbar(19661): onTabChanged: FAVICON
D/GeckoToolbar(19661): setFavicon(android.graphics.Bitmap@43476730)
D/GeckoBrowserApp(19661): BrowserApp.onTabChanged: 0: FAVICON
D/GeckoToolbar(19661): setFavicon(android.graphics.Bitmap@43476730)
D/GeckoToolbar(19661): Ignoring favicon set: new favicon is identical to previous favicon.
On access, both tabs open they both do not update their favicons (see screenshot)
D/GeckoBrowserApp(19661): BrowserApp.onTabChanged: 5: FAVICON
D/GeckoToolbar(19661): setFavicon(android.graphics.Bitmap@4389b578)
D/GeckoToolbar(19661): Ignoring favicon set: new favicon is identical to previous favicon.
Comment 14•11 years ago
|
||
The fact this is only reproducible when you update from 26 to 27 suggests it might be a bug in one of the many updateDatabaseFromXToY functions in BrowserProvider.java.
But, the latest of those is upgradeDatabaseFrom16to17, added in July 2013 (that's before I came along and started poking favicons with a stick). It's deleting favicons with zero-length payloads - that shouldn't cause a problem here.
The not-updating-on-load-despite-valid-icon-existing-on-the-internet problem suggests whatever failure caused them to not load in the first place had them stuck into the failed cache. (Which never, ever clears during a session. There's a bug about that. (Although possibly not a bug..? Another story for another time.)).
This view uses getSizedFaviconForPageFromLocal to load favicons. This sets the mOnlyFromLocal switch in LoadFaviconTask. This switch incorrectly causes the task to abort after checking the database but before loading from the jar. RNewman - did you do this on purpose, or is that a bug?
That certainly would cause favicons that are in jars but not the database to fail to be loaded. It's also against the spirit of the mOnlyFromLocal flag - so I suggest the flag either wants renaming to reflect its actual semantics, or that the jar should be checked in these cases.
I hope some of the above is, at worst, almost but not quite entirely unuseful in resolving this bug.
Flags: needinfo?(chriskitching)
Assignee | ||
Comment 15•11 years ago
|
||
My speculation: the favicon URI for the distribution bookmarks changed, but of course we don't re-process the distribution file.
Assignee | ||
Comment 16•11 years ago
|
||
(In reply to Chris Kitching [:ckitching] from comment #14)
> But, the latest of those is upgradeDatabaseFrom16to17, added in July 2013
> (that's before I came along and started poking favicons with a stick). It's
> deleting favicons with zero-length payloads - that shouldn't cause a problem
> here.
This looks like it might be the problem. I just looked in my own Nightly profile's DB and immediately found an entry with a broken favicon foreign key relation. (Not one of these...)
sqlite> select favicon_id from bookmarks where title = 'FAQ''s & Support';
favicon_id
----------------------------------------
1109
sqlite> select count(*) from favicons where _id = 1109;
count(*)
----------------------------------------
0
> This view uses getSizedFaviconForPageFromLocal to load favicons. This sets
> the mOnlyFromLocal switch in LoadFaviconTask. This switch incorrectly causes
> the task to abort after checking the database but before loading from the
> jar. RNewman - did you do this on purpose, or is that a bug?
These icons are data: URIs. It's nothing to do with JAR loading.
Assignee | ||
Comment 17•11 years ago
|
||
Aaron, could you grab browser.db from each profile before and after upgrade? You can use
https://addons.mozilla.org/en-US/android/addon/copy-profile/
to do it.
Flags: needinfo?(aaron.train)
Comment 18•11 years ago
|
||
Flags: needinfo?(aaron.train)
Assignee | ||
Comment 19•11 years ago
|
||
Thanks, Aaron.
Here's the difference.
26: bookmarks table has no favicon_ids or favicon_urls. favicons table is empty.
27: bookmarks table has favicon_ids 1, 2, 3. favicons table contains ids 1, 2, 3, 5, each apparently PNGs. Only one has a URL, which I deduce is from Aaron visiting SUMO.
I have no idea how 26 is able to display these favicons at all, 'cos the data ain't in the DB.
In 27, the data seems to be there, so the implication is that one of these things is occurring:
* The displayed bookmark somehow isn't being found in the DB.
* The join between bookmark and favicon isn't working.
* We're failing to decode the retrieved icon.
* Something else.
Updated•11 years ago
|
Assignee: nobody → rnewman
tracking-fennec: ? → 27+
Updated•11 years ago
|
Status: REOPENED → ASSIGNED
Assignee | ||
Comment 20•11 years ago
|
||
It's vanishingly unlikely that I'll make progress on this before 27 ships. If that bugs y'all, let me know (and ideally find another assignee).
If this applies to user-added bookmarks I'd be much more concerned. pcheng, Aaron, does it?
Flags: needinfo?(pcheng)
Reporter | ||
Comment 21•11 years ago
|
||
(In reply to Richard Newman [:rnewman] from comment #20)
> It's vanishingly unlikely that I'll make progress on this before 27 ships.
> If that bugs y'all, let me know (and ideally find another assignee).
>
> If this applies to user-added bookmarks I'd be much more concerned. pcheng,
> Aaron, does it?
User-added bookmarks does not have this problem.
Flags: needinfo?(pcheng)
Comment 23•11 years ago
|
||
Assume you mean 28, which ships week of the 18th.
Assignee | ||
Comment 24•11 years ago
|
||
Yes, thanks for the catch. This bug was tracking 27+, which twisted my brain.
Given that this bug affects upgrades from 26 to 27, we might just RESOLVED OHWELL this...
Assignee | ||
Comment 26•11 years ago
|
||
Horse is gone, won't spend time bolting the barn door.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → WONTFIX
Updated•11 years ago
|
Keywords: regressionwindow-wanted
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•