1007300 - Rendering of the "Not Now" menu item in popup notifications is messed up

Status: VERIFIED FIXED

(Toolkit :: UI Widgets, defect)

Description

[[:Aleksej]]

Steps to reproduce:
1. Save attachment 8418090 [details].
2. Open it in Firefox.
3. Click the field with "Enter your address".
4. In the doorhanger, click the down arrow near "Share Location".

Actual results:
The menu item that appears contains only a red close button.

Expected results:
The menu item contains text "Not Now".


When the page is opened from the Web, the menu contains (note the differing alignment of Not Now):
*  "Always Share Location"
*  "Never Share Location"
-------------------------
* "Not Now [x]"



In both cases here, the menu has dark background here, like normal drop-down menus (a Shiki color theme).


Regression range:
good 2014-04-25-03-02-09-mozilla-central-firefox-31.0a1.en-US.linux-x86_64 b836d89be72b
bad  2014-04-26-03-02-04-mozilla-central-firefox-31.0a1.en-US.linux-x86_64 0e91262606a6
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b836d89be72b&tochange=0e91262606a6

A smaller range:
WFM with 20140425130409 https://hg.mozilla.org/integration/mozilla-inbound/rev/cfe59efce491
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=cfe59efce491&tochange=0e91262606a6

Comment 1

[:Gijs (he/him)]

Could you try to manually narrow down the range using TBPL and the fx-team tree, instead of mozilla-inbound? I suspect the breaking change landed there. Here's a link:

https://tbpl.mozilla.org/?tree=Fx-Team&fromchange=1c2ef0d3d997&tochange=fcf19894d9f3

You can click the "B" next to Linux / Linux 64 (depending on whether you need 32 or 64 bit binaries) in the rows labelled "Opt" or "PGO" (DON'T use the ones labelled "Debug" because they're really slow) and then click "go to build directory" at the bottom left to find a tarball with the right build.


(Unfortunately mozregression doesn't do this for you, it's being discussed here: https://github.com/mozilla/mozregression/issues/79 but so far nobody has managed to make it happen. :-( )

Comment 2

[[:Aleksej]]

-g http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/fx-team-linux64/1398428049/
-b http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/fx-team-linux64/1398429071/

https://tbpl.mozilla.org/?tree=Fx-Team&fromchange=e91a5803fa7a&tochange=1f5f1fe135b9

    1f5f1fe135b9 Dão Gottwald – Bug 865826 - Remove the browser.tabs.closeButtons pref. r=jaws

Comment 3

[:Gijs (he/him)]

(In reply to [:Aleksej] from comment #2)
> -g
> http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/fx-team-
> linux64/1398428049/
> -b
> http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/fx-team-
> linux64/1398429071/
> 
> https://tbpl.mozilla.org/?tree=Fx-
> Team&fromchange=e91a5803fa7a&tochange=1f5f1fe135b9
> 
>     1f5f1fe135b9 Dão Gottwald – Bug 865826 - Remove the
> browser.tabs.closeButtons pref. r=jaws

Thanks!

Comment 4

[Tim Nguyen :ntim]

I can't seem to reproduce on Latest Nightly (on Windows 8.1)

Comment 5

[:Gijs (he/him)]

(In reply to Tim Nguyen [:ntim] from comment #4)
> I can't seem to reproduce on Latest Nightly (on Windows 8.1)

I expect it to be a Linux theme issue, although I can't be sure without actually checking what's going on on Linux.

Comment 6

[:Gijs (he/him)]

(In reply to :Gijs Kruitbosch from comment #1)
> (Unfortunately mozregression doesn't do this for you, it's being discussed
> here: https://github.com/mozilla/mozregression/issues/79 but so far nobody
> has managed to make it happen. :-( )

Aleksej pointed out to me this link is dead. I think it's now tracked in bug 997322.

Comment 7

[Dão Gottwald [:dao]]

Attachment: patch

close-icon should only be set on elements that /are/ close buttons rather than on elements want to embed a close icon somewhere. The latter doesn't work with the Linux theme in particular, where close-icon is implemented using background-image instead of list-style-image.

Comment 8

[Dão Gottwald [:dao]]

https://hg.mozilla.org/integration/fx-team/rev/4d044c48fd97

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4d044c48fd97

Comment 11

[Dão Gottwald [:dao]]

Comment on attachment 8419944 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 865826
User impact if declined: see comment 0
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): no risk
String or IDL/UUID changes made by this patch: none

Comment 12

[Dão Gottwald [:dao]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/42d103fc8993

Comment 13

[Francesca Ciceri [:madamezou]]

Hi,

I was able to reproduce it on Mozilla/5.0 (X11; Linux x86_64; rv:32.0) Gecko/20100101 Firefox/32.0 ID:20140507030202 and I can confirm it's fixed on latest Beta (Firefox/31.0 ID:20140619131915) as well as on latest Aurora (Firefox/32.0 ID:20140620004003).

Still some weird behaviour of the geolocation window, but that's related to https://bugzilla.mozilla.org/show_bug.cgi?id=58441.

Cheers,
Francesca

Comment 14

[Marco Mucci [:MarcoM]]

Hi Dao, can you provide a point estimate and I'll add it to the current iteration.