700333 - Highlighter close button has a bogus hover state on Windows

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Dão Gottwald [:dao]]

Attachment: patch

Comment 1

[Mihai Sucan [:msucan]]

Shouldn't this be for Linux as well? The close button also has a weird hover state on my Ubuntu system.

Comment 2

[Dão Gottwald [:dao]]

Linux uses,instead of custom images, a stock icon, which is supposed to be used with the native toolbar button style.

Comment 3

[Mihai Sucan [:msucan]]

(In reply to Dão Gottwald [:dao] from comment #2)
> Linux uses,instead of custom images, a stock icon, which is supposed to be
> used with the native toolbar button style.

True. I see the native button graphics and icon when I hover - which looks broken in the current toolbar design.

Comment 4

[Rob Campbell [:rc] (:robcee)]

I saw this yesterday as well. Checking...

Comment 5

[Rob Campbell [:rc] (:robcee)]

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

Thanks for the patch!

Comment 6

[Rob Campbell [:rc] (:robcee)]

marking this as tracking firefox 10.

This is a styling fix for a single platform. Risk is negligible. It improves the appearance of the Highlighter's toolbar on Windows.

Comment 7

[Rob Campbell [:rc] (:robcee)]

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

asking for approval on aurora 10.

Comment 8

[Alex Keybl [:akeybl]]

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

[Triage Comment]
Given this is just a styling change, approving for aurora.

Comment 9

[Dão Gottwald [:dao]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/8e42e063903b

Comment 10

[Ed Morley [:emorley]]

Backed out of inbound for Mochitest failures on Windows:

https://tbpl.mozilla.org/php/getParsedLog.php?id=7314472&tree=Mozilla-Inbound
{
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_highlighter.js | transparent veil box width matches dimensions of element (no zoom) - Got 0, expected 22
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_highlighter.js | transparent veil box width matches width of element (2x zoom) - Got 937, expected 44
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_highlighter.js | transparent veil box height matches width of element (2x zoom) - Got 76, expected 44
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_iframeTest.js | selection matches div2 node - Got [object HTMLDivElement], expected [object HTMLDivElement]
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_iframeTest.js | highlighter matches selection - Got [object HTMLDivElement], expected [object HTMLDivElement] 
}

https://hg.mozilla.org/integration/mozilla-inbound/rev/f7ea68d2d546

Comment 11

[Dão Gottwald [:dao]]

What the hell... what are these tests doing?

Comment 12

[Rob Campbell [:rc] (:robcee)]

Did the patch apply cleanly when it went in? I had to do some tweaking to get it to apply on my machine for whatever reason. There's no way that should've affected those tests, but I will try to verify and renom for aurora.

Comment 13

[Dão Gottwald [:dao]]

(In reply to Rob Campbell [:rc] (robcee) from comment #12)
> Did the patch apply cleanly when it went in?

Yep. Bug 701212 landed in the meantime.

Comment 14

[Paul Rouget [:paul]]

I can't reproduce (central + this patch rebased).
I'll see how it goes with the try-server.

Comment 15

[Paul Rouget [:paul]]

Attachment: patch - rebased

Comment 16

[Paul Rouget [:paul]]

Realizing only now it's a Windows XP only failure: https://tbpl.mozilla.org/?tree=Try&rev=82c7ccd6242d

Comment 17

[Paul Rouget [:paul]]

Same problem with Linux here: bug 702776

Comment 18

[Paul Rouget [:paul]]

Now that bug 703237 is fixed, can we land this patch again?

Comment 19

[Rob Campbell [:rc] (:robcee)]

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

Comment 20

[Rob Campbell [:rc] (:robcee)]

https://hg.mozilla.org/mozilla-central/rev/caf7c101040e

Comment 21

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 574564 [details] [diff] [review]
patch - rebased

style only fix. requires test fix in bug 703237 to prevent test failures.

Comment 22

[Rob Campbell [:rc] (:robcee)]

patch doesn't apply cleanly due to bug 701212.

Comment 23

[Rob Campbell [:rc] (:robcee)]

aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/5f68d58cb3fd

Comment 24

[u279076]

What is the expected & unexpected hover states for QA to verify this fix?

Comment 25

[Paul Rouget [:paul]]

(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #24)
> What is the expected & unexpected hover states for QA to verify this fix?

Expected: on over, the close button of the inspector toolbar should be a bit different (darker iirc, see chrome://browser/skin/devtools/toolbarbutton-close.png)
Unexpected: on over, the close button use a totally wrong and unrelated image

Comment 26

[Paul Silaghi, QA [:pauly]]

I've verified this on Win XP and Win 7.
On hover, the inspector close button is red and on click gets darker.
Setting this to verified fix on Firefox 10b2.

What about the web console close button? On hover it becomes gray instead of red, although on click it turns into the same dark red as the inspector.