Closed Bug 1018913 Opened 10 years ago Closed 10 years ago

Customization mode favicon not showing anymore

Categories

(Firefox :: Theme, defect)

Product:
Firefox
Component:
Theme
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 33
Tracking Flags:
Tracking Status
firefox33 --- verified

People

(Reporter: u428464, Assigned: ttaubert)

References

Details

(Keywords: regression, reproducible, Whiteboard: p=1 s=33.1 [qa!])

Attachments

(1 file)

0001-Bug-1018913-Customization-mode-favicon-not-showing-a.patch
1.20 KB, patch
Gijs
: review+
Details | Diff | Splinter Review 
At least on Windows 7 the customization favicon is not showing anymore.
Keywords: regression
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
Seems to be a new issue on Nightly. I noticed it just today. I'll try to find more info.
Flags: needinfo?(ge3k0s)
Ok first bad build is 05.27.2014
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.
Summary: Customization mode favicon not showing anymore → Customization mode favicon not showing anymore on Win 7
Cant this be nominated for the backlog ?
(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?
Flags: firefox-backlog+
(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.
(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)
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)
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.
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e017c15325ae&tochange=1e712b724d17
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: firefox-backlog+
Keywords: regressionwindow-wanted → reproducible
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)
I can reproduce on mac as well.
OS: Windows 7 → All
Summary: Customization mode favicon not showing anymore on Win 7 → Customization mode favicon not showing anymore
I... don't know? Does backing out bug 1016942 fix it?
Flags: needinfo?(ttaubert)
(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...
Both of the builds above are already broken on my PC.
(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)
(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)
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)
Always me :'(
Flags: needinfo?(ttaubert)
Not sure why exactly that fixes it but I definitely forgot to update the file.
Attachment #8439398 - Flags: review?(gijskruitbosch+bugs)
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+
(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+]
(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
Closed: 10 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!]
Depends on: 1179739
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: