Customization mode favicon not showing anymore

VERIFIED FIXED in Firefox 33

Status

()

Firefox
Theme
VERIFIED FIXED
4 years ago
2 years ago

People

(Reporter: ge3k0s, Assigned: ttaubert)

Tracking

({regression, reproducible})

Trunk
Firefox 33
regression, reproducible
Points:
---
Dependency tree / graph
Bug Flags:
firefox-backlog +

Firefox Tracking Flags

(firefox33 verified)

Details

(Whiteboard: p=1 s=33.1 [qa!])

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
At least on Windows 7 the customization favicon is not showing anymore.
(Reporter)

Updated

4 years ago
Keywords: regression

Comment 1

4 years ago
More info? Nightly only? How long? Are you sure this isn't just the issue where it would not show in all the cases (but show in some) because something weird is up with our icon code?
Flags: needinfo?(ge3k0s)
Keywords: regressionwindow-wanted
(Reporter)

Comment 2

4 years ago
Seems to be a new issue on Nightly. I noticed it just today. I'll try to find more info.
Flags: needinfo?(ge3k0s)
(Reporter)

Comment 3

4 years ago
Ok first bad build is 05.27.2014
(Reporter)

Comment 4

4 years ago
I'm wondering if it's not the same issue that was present with Aero at the beginning : i.e. only Windows 7 had no favicon after the first patch landed.
(Reporter)

Updated

4 years ago
Summary: Customization mode favicon not showing anymore → Customization mode favicon not showing anymore on Win 7
(Reporter)

Comment 5

4 years ago
Cant this be nominated for the backlog ?

Comment 6

4 years ago
(In reply to Guillaume C. [:ge3k0s] from comment #3)
> Ok first bad build is 05.27.2014

(In reply to Guillaume C. [:ge3k0s] from comment #4)
> I'm wondering if it's not the same issue that was present with Aero at the
> beginning : i.e. only Windows 7 had no favicon after the first patch landed.

Now I'm just confused. Is this regression window still valid?

Updated

4 years ago
Flags: firefox-backlog+
(Reporter)

Comment 7

4 years ago
(In reply to :Gijs Kruitbosch from comment #6)
> (In reply to Guillaume C. [:ge3k0s] from comment #3)
> > Ok first bad build is 05.27.2014
> 
> (In reply to Guillaume C. [:ge3k0s] from comment #4)
> > I'm wondering if it's not the same issue that was present with Aero at the
> > beginning : i.e. only Windows 7 had no favicon after the first patch landed.
> 
> Now I'm just confused. Is this regression window still valid?

05.27.2014 is the first bad build on Windows 7. The other comment was just a theory since it seems to be an issue just on Win 7.

Comment 8

4 years ago
(In reply to Guillaume C. [:ge3k0s] from comment #7)
> (In reply to :Gijs Kruitbosch from comment #6)
> > (In reply to Guillaume C. [:ge3k0s] from comment #3)
> > > Ok first bad build is 05.27.2014
> > 
> > (In reply to Guillaume C. [:ge3k0s] from comment #4)
> > > I'm wondering if it's not the same issue that was present with Aero at the
> > > beginning : i.e. only Windows 7 had no favicon after the first patch landed.
> > 
> > Now I'm just confused. Is this regression window still valid?
> 
> 05.27.2014 is the first bad build on Windows 7. The other comment was just a
> theory since it seems to be an issue just on Win 7.

I tried to get a pushlog / proper regression window for this using mozregression on my win7 vm, but I can't reproduce.

What icon is missing? What do you see instead? I'm assuming this is about the icon shown on the tab, rather than the [() Nightly] identity block? How are you opening customize mode?

On the May 27 build I still see an icon there. Are you checking with a clean profile?

Clearing backlog flag while we're not sure what's going on.
Flags: firefox-backlog+ → needinfo?(ge3k0s)
(Reporter)

Comment 9

4 years ago
The customization favicon (in tab) is missing. Since we don't show the default favicon when there is none I see just the tab title. You're right the favicon is still there in 05.27.2014 build. It's my mistake. First bad build is 05.29.2014 build. Tested with a clean profile.
Flags: needinfo?(ge3k0s)
(Reporter)

Comment 10

4 years ago
Ok the favicon is there when you directly type about:customizing in the address bar, but is missing when you click on the menu panel button.

Comment 11

4 years ago
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e017c15325ae&tochange=1e712b724d17
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: firefox-backlog+
Keywords: regressionwindow-wanted → reproducible

Comment 13

4 years ago
And which blames https://tbpl.mozilla.org/?fromchange=e017c15325ae&tochange=1e712b724d17&rev=c8252fd40ba7, which means I put my money on bug 1016942.

Tim, is it possible/plausible that bug 1016942 broke this because we have the whole cached-like-about:newtab thingymajig going on for aboutCustomizing as well?
Flags: needinfo?(ttaubert)

Comment 14

4 years ago
I can reproduce on mac as well.
OS: Windows 7 → All

Updated

4 years ago
Summary: Customization mode favicon not showing anymore on Win 7 → Customization mode favicon not showing anymore
(Assignee)

Comment 15

4 years ago
I... don't know? Does backing out bug 1016942 fix it?
Flags: needinfo?(ttaubert)

Comment 16

4 years ago
(In reply to Tim Taubert [:ttaubert] from comment #15)
> I... don't know? Does backing out bug 1016942 fix it?

Interestingly... no. I must have made a mistake with the regression hunting because I /thought/ that it was working on:

http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-win32-pgo/1401305412/

( 9506880e4879 )

and broken on:

http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-win32-pgo/1401316203/

( c8252fd40ba7 )


I need to run out right now - Guillaume, do you have time to check if you get the same results? Almost everything else in that push is android-only, so I'm confused as to why I saw what I think I saw...
(Reporter)

Comment 17

4 years ago
Both of the builds above are already broken on my PC.

Comment 18

4 years ago
(In reply to Guillaume C. [:ge3k0s] from comment #17)
> Both of the builds above are already broken on my PC.

Hrm. New checks suggest https://hg.mozilla.org/mozilla-central/pushloghtml?startID=26845&endID=26846 instead, which is odd because it's an m-i merge... I'll bisect further on m-i in a second, but meanwhile...

Because I'm paranoid now, can you check that http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-win32/1401254569/ is broken and http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-win32/1401244733/ works?
Flags: needinfo?(ge3k0s)
(Reporter)

Comment 19

4 years ago
(In reply to :Gijs Kruitbosch from comment #18)
> Because I'm paranoid now, can you check that
> http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-
> central-win32/1401254569/ is broken and
> http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-
> central-win32/1401244733/ works?

This time this is correct. ;-)
Flags: needinfo?(ge3k0s)

Comment 20

4 years ago
Sooo... confirmed with a local backout, this was caused by... drumroll... bug 990812!

I expect that there's /something/ wrong with the frame script stuff causing the tab preloader to work... differently? But I don't pretend to understand /why/ :-\
Blocks: 990812
Flags: needinfo?(ttaubert)
(Assignee)

Comment 22

4 years ago
Always me :'(
Flags: needinfo?(ttaubert)
(Assignee)

Comment 23

4 years ago
Created attachment 8439398 [details] [diff] [review]
0001-Bug-1018913-Customization-mode-favicon-not-showing-a.patch

Not sure why exactly that fixes it but I definitely forgot to update the file.
Attachment #8439398 - Flags: review?(gijskruitbosch+bugs)

Comment 24

4 years ago
Comment on attachment 8439398 [details] [diff] [review]
0001-Bug-1018913-Customization-mode-favicon-not-showing-a.patch

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

Do we have a bug already for unifying these two files, and if not, can you file one and CC me + mconley? Thanks!

(might be useful to do a diff to find any other things that may have sneaked in if you haven't already...)
Attachment #8439398 - Flags: review?(gijskruitbosch+bugs) → review+

Comment 25

4 years ago
(In reply to Tim Taubert [:ttaubert] from comment #22)
> Always me :'(

(thanks for picking this up so quickly!)

Marco, I think Tim is fixing this this iteration. ;-)
Assignee: nobody → ttaubert
Status: NEW → ASSIGNED
Flags: needinfo?(mmucci)
Whiteboard: p=1 [qa+]
(In reply to :Gijs Kruitbosch from comment #24)
> Comment on attachment 8439398 [details] [diff] [review]
> 0001-Bug-1018913-Customization-mode-favicon-not-showing-a.patch
> 
> Review of attachment 8439398 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Do we have a bug already for unifying these two files, and if not, can you
> file one and CC me + mconley? Thanks!

We _could_ file that, but I'd prefer following up on bug 1014185 and getting rid of the about:customizing preloading hack entirely.
Added to Iteration 33.1
Flags: needinfo?(mmucci)
Whiteboard: p=1 [qa+] → p=1 s=33.1 [qa+]
(Assignee)

Comment 28

4 years ago
(In reply to Mike Conley (:mconley) from comment #26)
> We _could_ file that, but I'd prefer following up on bug 1014185 and getting
> rid of the about:customizing preloading hack entirely.

I agree, I hope we can get rid of about:newtab preloading too in the future.
https://hg.mozilla.org/mozilla-central/rev/8c4371b5482e
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 33
QA Contact: camelia.badau
Verified fix on Windows 7 64bit, Ubuntu 13.10 32bit and Mac OSX 10.8.5 using latest Nightly 33.0a1 (buildID: 20140615030204).
Status: RESOLVED → VERIFIED
status-firefox33: --- → verified
Whiteboard: p=1 s=33.1 [qa+] → p=1 s=33.1 [qa!]

Updated

2 years ago
Depends on: 1179739
You need to log in before you can comment on or make changes to this bug.