Some icons are not displayed in the bookmarks sidebar

RESOLVED FIXED in Firefox 38

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

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

Tracking

43 Branch
mozilla39
x86
Windows XP
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox36 wontfix, firefox37 wontfix, firefox38 fixed, firefox39 fixed)

Details

(Whiteboard: gfx-noted)

Attachments

(1 attachment)

Reporter

Description

4 years ago
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.
Reporter

Comment 1

4 years ago
Also affected 37 and 38 versions as far as I understand.
Reporter

Updated

4 years ago
Component: Untriaged → Bookmarks & History

Comment 2

4 years ago
Also icons disappears in ScrapBook addons and bookmarks.

PS Developer Scrapbook X inform, that because new API applyed in FF36??
Reporter

Comment 3

4 years ago
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/
Reporter

Comment 4

4 years ago
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.

Comment 5

4 years ago
And what about Scrapbook icons?

PS how i can change ICO>PNG through this addon? https://addons.mozilla.org/uk/firefox/addon/sqlite-manager/
Reporter

Comment 6

4 years ago
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.

Comment 7

4 years ago
Вы говорите по русски?)

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

Вы простите. Нельзя ли по подробнее чем открыть places.sqlite и как поменять image/x-icon в image/png.
Reporter

Comment 8

4 years ago
Вы мне е-мэйлом бы писали тогда, а то нерусскоязычные точно будут эту запись игнорировать :)

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

А вот SQLITE manager-ом и открыть. Там залезаете в таблицу moz_favicons, ищете свою иконку, сохраняете её (поле data) в файл, конвертируете этот файл чем-нибудь в PNG (у меня, скажем, GIMP), и импортируете обратно. А в поле mime_type просто текст - его и надо с "image/x-icon" на "image/png" поменять. Операция, увы, поштучная.. я, во всяком случае, не нашёл никаких пакетных файловых операций в SM.

Comment 9

4 years ago
Ясно, спасибо). Буду пробовать. Жаль, что целиком нельзя все поменять(
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

Comment 11

4 years ago
(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
Assignee

Comment 12

4 years ago
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)

Comment 13

4 years ago
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.
Assignee

Comment 14

4 years ago
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)
Assignee

Comment 15

4 years ago
Posted patch patchSplinter 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: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39

Comment 20

4 years ago
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.

Comment 24

4 years ago
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)

Comment 30

4 years ago
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.
Reporter

Comment 32

4 years ago
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 :).
Reporter

Comment 33

4 years ago
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
Reporter

Comment 34

4 years ago
Confirmed in 43.0 - 43.0.3
Reporter

Comment 35

4 years ago
All favicons must be converted in the PNG format, all it's neverendless story.
Reporter

Comment 36

4 years ago
"all it's" == "otherwise it's", sorry
You need to log in before you can comment on or make changes to this bug.