Closed Bug 1137615 Opened 6 years ago Closed 6 years ago

Some icons are not displayed in the bookmarks sidebar

Categories

(Core :: ImageLib, defect)

Product:
Core
Component:
ImageLib
Version:
43 Branch
Platform:
x86
Windows XP
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla39
Tracking Flags:
Tracking Status
firefox36 --- wontfix
firefox37 --- wontfix
firefox38 --- fixed
firefox39 --- fixed

People

(Reporter: andrew.i.r, Assigned: euthanasia_waltz)

References

Details

(Whiteboard: gfx-noted)

Attachments

(1 file)

patch
2.32 KB, patch
seth
: review+
Sylvestre
: approval-mozilla-aurora+
Sylvestre
: approval-mozilla-beta-
Details | Diff | Splinter Review 
User Agent: Mozilla/5.0 (Windows NT 5.1; rv:35.0) Gecko/20100101 Firefox/35.0
Build ID: 20150122214805

Steps to reproduce:

1. Open the bookmarks sidebar (Ctrl-B)
2. Create a new folder in the Bookmarks Menu.
3. Copy (just copy / paste) into this folder following URLs (it's only sample, problem occured with some other URLs too):
---
http://forum.guns.ru/forumtopics/143.html
http://forum.guns.ru/forumtopics/342.html
http://forum.guns.ru/forumtopics/94.html
http://forum.guns.ru/forumtopics/343.html
http://forum.guns.ru/forumtopics/64.html
http://forum.guns.ru/forumtopics/5.html
http://forum.guns.ru/forumtopics/308.html
http://forum.guns.ru/forumtopics/182.html
http://forum.guns.ru/forumtopics/200.html
http://forum.guns.ru/forummessage/84/98822.htm
http://forum.guns.ru/forumindex/7.html
---

4. Open all these bookmarks to get favicons.
5. Close opened tabs, close the folder (!), restart Firefox.
6. Open the folder.


Actual results:

A blank space instead of icons. The icons appear when you move the mouse over the bookmarks.


Expected results:

Icons instead of blank space.
Also affected 37 and 38 versions as far as I understand.
Component: Untriaged → Bookmarks & History
Also icons disappears in ScrapBook addons and bookmarks.

PS Developer Scrapbook X inform, that because new API applyed in FF36??
It seems that the problem occurs with sites having image/x-icon favicon.
Other sites having this issue,for example.
http://www.vmware.com/
http://slashdot.jp/
http://gigazine.net/
http://news.mynavi.jp/
http://www.iiyama.co.jp/
http://jpn.nec.com/
Confirmation of this version is the fact that a direct replacement icons from ICO to PNG (places.sqlite->moz_favicons->data and mime_type) solves the problem. And vice versa - replacement from PNG to ICO reproduces the effect of invisibility.
And what about Scrapbook icons?

PS how i can change ICO>PNG through this addon? https://addons.mozilla.org/uk/firefox/addon/sqlite-manager/
I do not use SB, sorry.

For changing FF favicons, you must export places.sqlite->moz_favicons->data as file, convert it, using any graphics editor and import back to same place. Also you must change places.sqlite->moz_favicons->mime_type from image/x-icon to image/png.
Вы говорите по русски?)

Сделал как Вы говорили для теста с пропаданием, у меня все иконки на месте. Но проподяют в Скепбуке каждый раз как захожу в него, нужно провести мышкой по ним или открыть\закрыть папки.

Вы простите. Нельзя ли по подробнее чем открыть places.sqlite и как поменять image/x-icon в image/png.
Вы мне е-мэйлом бы писали тогда, а то нерусскоязычные точно будут эту запись игнорировать :)

Я говорю, что не знаком со Скрэпбуком лично, следовательно ничего про него сказать не могу, увы.

А вот SQLITE manager-ом и открыть. Там залезаете в таблицу moz_favicons, ищете свою иконку, сохраняете её (поле data) в файл, конвертируете этот файл чем-нибудь в PNG (у меня, скажем, GIMP), и импортируете обратно. А в поле mime_type просто текст - его и надо с "image/x-icon" на "image/png" поменять. Операция, увы, поштучная.. я, во всяком случае, не нашёл никаких пакетных файловых операций в SM.
Ясно, спасибо). Буду пробовать. Жаль, что целиком нельзя все поменять(
This doesn't seem like a problem with the bookmarks system itself, but rather some problem with the rendering.
Component: Bookmarks & History → Graphics
Product: Firefox → Core
(In reply to Luís Miguel [:Quicksaver] from comment #10)
> This doesn't seem like a problem with the bookmarks system itself, but
> rather some problem with the rendering.

And what we need to do?

ps https://youtu.be/bygYl6-2jHI
Platforms in addition.
Windows 8.1 x64
Windows 7 Home Premium x64
Linux Mint 17.1 Cinnamon 64-bit
Linux Mint 17.1 Cinnamon 32-bit

There is a bit different behaviour on Linux.
The problematic icon is not displayed when I opened(expanded) the bookmark folder, but it is displayed just when I move mouse pointer outside the bookmarks tree area.
(other triggers - mousehover, close/reopen the bookmark folder, expand another bookmark folder, start Firefox with expanded bookmark folder - are same as Windows)
I would also like to confirm this bug on the following OS (on 4 different hardware configs):
Windows 7 Professional x64
Windows 7 Home Premium x64
Linux Mint 17.1 Cinnamon 32-bit

For Linux Mint, a new, unmodified install was used to confirm this bug.

The bug is present in 36 and 36.0.1. I never saw it in older versions and did not check with newer builds.

The defective favicons seem to be limited to "image/x-icon".

This bug may not break FF, but it is quite ugly to have icons pop in all the time in the sidebar after every FF start.
firefox-36.0.1.source.tar.bz2 and firefox-36.0.4.source.tar.bz2
in 'image' dir
decoders/nsICODecoder.cpp:88:    mInvalidRect.Union(mContainedDecoder->TakeInvalidRect());
decoders/nsICODecoder.cpp:602:  mInvalidRect.Union(mContainedDecoder->TakeInvalidRect());
decoders/nsICODecoder.cpp:650:    mInvalidRect.Union(mContainedDecoder->TakeInvalidRect());
src/RasterImage.cpp:2659:      invalidRect.Union(decoder->TakeInvalidRect());
src/RasterImage.cpp:2685:    mNotifyInvalidRect.Union(invalidRect);

should be
decoders/nsICODecoder.cpp:88:    mInvalidRect = mInvalidRect.Union(mContainedDecoder->TakeInvalidRect());
decoders/nsICODecoder.cpp:602:  mInvalidRect = mInvalidRect.Union(mContainedDecoder->TakeInvalidRect());
decoders/nsICODecoder.cpp:650:    mInvalidRect = mInvalidRect.Union(mContainedDecoder->TakeInvalidRect());
src/RasterImage.cpp:2659:      invalidRect = invalidRect.Union(decoder->TakeInvalidRect());
src/RasterImage.cpp:2685:    mNotifyInvalidRect = mNotifyInvalidRect.Union(invalidRect);

(compiled and confirmed/verified on LinuxMint)
Attached patch patch — — Splinter Review
Flags: needinfo?(seth)
Whiteboard: gfx-noted
Component: Graphics → ImageLib
Attachment #8582935 - Flags: review?(seth)
Comment on attachment 8582935 [details] [diff] [review]
patch

Review of attachment 8582935 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for tracking this down.

We need to ensure that we get a warning when the return value of a method like Union() is unused. We should have caught this automatically.
Attachment #8582935 - Flags: review?(seth) → review+
Flags: needinfo?(seth)
(In reply to Seth Fowler [:seth] from comment #16)
> We need to ensure that we get a warning when the return value of a method
> like Union() is unused. We should have caught this automatically.

To follow up: in bug 1147706 I'm adding annotations that will make us warn when the return value of Union() and similar methods is ignored. I've confirmed that they would have caught this bug.
https://hg.mozilla.org/mozilla-central/rev/8ab8a64af8ec
Status: UNCONFIRMED → RESOLVED
Closed: 6 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
And how fix in 36.0.4?
Looks like bug 1100725 added the problematic code.
Blocks: 1100725
Comment on attachment 8582935 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: bug 1100725
[User impact if declined]: images whose filetype is icon may not draw properly, like in the bookmarks bar
[Describe test coverage new/current, TreeHerder]: nothing specifically
[Risks and why]: low risk
[String/UUID change made/needed]: none
Attachment #8582935 - Flags: approval-mozilla-beta?
Attachment #8582935 - Flags: approval-mozilla-aurora?
(In reply to a.v.procenko from comment #20)
> And how fix in 36.0.4?

Generally for small fixes like this we don't make a new release on the release channel. (ie no 36.0.5) But I've nominated the fix for beta and aurora. It's most likely too late for 37 as that gets released on tuesday. If you want to use Firefox with the fix you can either use Firefox Nightly now, wait until the first Firefox beta 38 comes out in a week or so, use Fireox Aurora (developer edition) when the fix gets uplifted there (likely less than a week), or wait until 38 comes out on May 11.
I try 39 nightly, and i have same icon disappear in Scarpbook that i have in 36.0.4(((
(In reply to a.v.procenko from comment #24)
> I try 39 nightly, and i have same icon disappear in Scarpbook that i have in
> 36.0.4(((

Yes, the patch didn't make the last Nightly build. It only landed this morning. It should be in tonight's Nightly.
Comment on attachment 8582935 [details] [diff] [review]
patch

Too late for 37.
Attachment #8582935 - Flags: approval-mozilla-beta?
Attachment #8582935 - Flags: approval-mozilla-beta-
Attachment #8582935 - Flags: approval-mozilla-aurora?
Attachment #8582935 - Flags: approval-mozilla-aurora+
(In reply to Seth Fowler [:seth] from comment #18)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/8ab8a64af8ec

In the future, please be more careful about double-checking commit information for submitted patches.
Assignee: nobody → euthanasia_waltz
Flags: needinfo?(seth)
Talked with Ryan about the issue with the commit information in that patch, which I didn't immediately notice.

Andrew, you may want to set a valid email address for usage with hg.
Flags: needinfo?(seth)
in aurora FF38 Scrapbook icons working good.
but what with FF36? waiting until FF38?
(In reply to a.v.procenko from comment #30)
> in aurora FF38 Scrapbook icons working good.
> but what with FF36? waiting until FF38?

Unfortunately the fix won't arrive until FF38, yeah. (Which means 6 weeks until it lands in a release.) The change just missed FF37.
I confirm that it's fixed in version 38 beta.
The question is closed, thanks to all who took part in the discussion (and all developers, of course :).
Version 43 has the same or a similar error(42 it was not). If plaсes.sglite-> moz_faviсons-> mime_tipe is image/PNG, and the date is gif file, icon only appears when holding the mouse cursor over it. If the file in the database manyally replaced by true PNG - the effect disappears.
Version: 36 Branch → 43 Branch
Confirmed in 43.0 - 43.0.3
All favicons must be converted in the PNG format, all it's neverendless story.
"all it's" == "otherwise it's", sorry
You need to log in before you can comment on or make changes to this bug.