Last Comment Bug 700333 - Highlighter close button has a bogus hover state on Windows
: Highlighter close button has a bogus hover state on Windows
Status: RESOLVED FIXED
[qa+][qa!:10]
: verified-beta
Product: Firefox
Classification: Client Software
Component: Developer Tools (show other bugs)
: Trunk
: x86 Windows XP
: -- trivial (vote)
: Firefox 11
Assigned To: Rob Campbell [:rc] (:robcee)
:
: J. Ryan Stinnett [:jryans] (use ni?)
Mentors:
Depends on: 701212 703237
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-07 09:13 PST by Dão Gottwald [:dao]
Modified: 2012-02-01 13:57 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
verified


Attachments
patch (1.32 KB, patch)
2011-11-07 09:13 PST, Dão Gottwald [:dao]
rcampbell: review+
Details | Diff | Splinter Review
patch - rebased (1.29 KB, patch)
2011-11-15 03:24 PST, Paul Rouget [:paul]
christian: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Dão Gottwald [:dao] 2011-11-07 09:13:52 PST
Created attachment 572494 [details] [diff] [review]
patch
Comment 1 Mihai Sucan [:msucan] 2011-11-07 09:23:57 PST
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] 2011-11-07 09:41:46 PST
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] 2011-11-07 09:44:14 PST
(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) 2011-11-08 07:45:44 PST
I saw this yesterday as well. Checking...
Comment 5 Rob Campbell [:rc] (:robcee) 2011-11-08 10:19:30 PST
Comment on attachment 572494 [details] [diff] [review]
patch

Thanks for the patch!
Comment 6 Rob Campbell [:rc] (:robcee) 2011-11-08 10:22:02 PST
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) 2011-11-08 10:22:29 PST
Comment on attachment 572494 [details] [diff] [review]
patch

asking for approval on aurora 10.
Comment 8 Alex Keybl [:akeybl] 2011-11-08 14:53:20 PST
Comment on attachment 572494 [details] [diff] [review]
patch

[Triage Comment]
Given this is just a styling change, approving for aurora.
Comment 10 Ed Morley [:emorley] 2011-11-09 13:25:52 PST
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] 2011-11-09 15:27:04 PST
What the hell... what are these tests doing?
Comment 12 Rob Campbell [:rc] (:robcee) 2011-11-14 07:59:14 PST
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] 2011-11-14 08:04:53 PST
(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] 2011-11-15 03:19:33 PST
I can't reproduce (central + this patch rebased).
I'll see how it goes with the try-server.
Comment 15 Paul Rouget [:paul] 2011-11-15 03:24:29 PST
Created attachment 574564 [details] [diff] [review]
patch - rebased
Comment 16 Paul Rouget [:paul] 2011-11-15 06:37:38 PST
Realizing only now it's a Windows XP only failure: https://tbpl.mozilla.org/?tree=Try&rev=82c7ccd6242d
Comment 17 Paul Rouget [:paul] 2011-11-17 03:23:47 PST
Same problem with Linux here: bug 702776
Comment 18 Paul Rouget [:paul] 2011-11-21 15:19:16 PST
Now that bug 703237 is fixed, can we land this patch again?
Comment 19 Rob Campbell [:rc] (:robcee) 2011-11-24 05:27:08 PST
https://hg.mozilla.org/integration/fx-team/rev/caf7c101040e
Comment 20 Rob Campbell [:rc] (:robcee) 2011-11-24 08:26:18 PST
https://hg.mozilla.org/mozilla-central/rev/caf7c101040e
Comment 21 Rob Campbell [:rc] (:robcee) 2011-11-24 08:31:40 PST
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) 2011-11-29 08:37:33 PST
patch doesn't apply cleanly due to bug 701212.
Comment 23 Rob Campbell [:rc] (:robcee) 2011-11-30 06:13:32 PST
aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/5f68d58cb3fd
Comment 24 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-12-28 14:20:36 PST
What is the expected & unexpected hover states for QA to verify this fix?
Comment 25 Paul Rouget [:paul] 2012-01-03 02:54:34 PST
(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] 2012-01-04 06:06:25 PST
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.

Note You need to log in before you can comment on or make changes to this bug.