User-Agent: Build Identifier: Current winstripe tabbrowser icons (not tab style) needs to be updated. Reproducible: Always

Patch with fixed jar.mn.

Comment on attachment 449282 [details] [diff] [review] patch v2 > .tab-close-button { > -moz-appearance: none; >- -moz-image-region: rect(0px, 56px, 14px, 42px); >+ -moz-image-region: rect(0px, 64px, 16px, 48px); > border: none; > padding: 0px; >@@ -1091,6 +1091,6 @@ > border: none; > padding: 0px; >- list-style-image: url("chrome://global/skin/icons/close.png"); >+ list-style-image: url("chrome://browser/skin/tabbrowser/close.png"); > } Are you sure you don't just want to update chrome://global/skin/icons/close.png with the new image?

(In reply to comment #4) > Are you sure you don't just want to update chrome://global/skin/icons/close.png > with the new image? I'm not sure that other apps like Thunderbird want to use it.

Thunderbird can always copy the old image if they want to. I think the really relevant question is, should notification.css use the old or the new image? Also, killing closeSidebar.png in favor of the new image might make sense.

I don't know, it can be changed after, but now this patch better solution. It should be discussed with UX team (or with shorlander).

(In reply to comment #7) > I don't know, it can be changed after, but now this patch better solution. I would agree if you just maintained preexisting inconsistency. But since you're actually adding a new image, I think replacing the existing close.png is preferable.

Comment on attachment 449536 [details] [diff] [review] patch v3 The CSS changes seem to be missing here. I assume you still changed the size of the close icon.

(In reply to comment #10) > (From update of attachment 449536 [details] [diff] [review]) > The CSS changes seem to be missing here. I assume you still changed the size of > the close icon. No, I just removed some spaces between close icons. CSS changes unneeded here.

Ok. Another thing: I see you removed the -aero images in browser/ but not in toolkit/. Intentional?

(In reply to comment #12) > Ok. Another thing: I see you removed the -aero images in browser/ but not in > toolkit/. Intentional? Yes. Probably close icons will have variants for aero and luna in the future.

I'm unable to apply your latest patch: patching file browser/themes/winstripe/browser/jar.mn Hunk #1 FAILED at 101 Hunk #2 FAILED at 160 Hunk #3 FAILED at 165 ** Unbekannter Fehler, Details folgen ** Problemdetails bitte bei http://www.selenic.com/mercurial/bts ** oder mercurial@selenic.com melden ** Mercurial Distributed SCM (version 1.4.3) ** Erweiterungen geladen: rebase Traceback (most recent call last): File "/usr/bin/hg", line 27, in <module> mercurial.dispatch.run() File "/usr/lib/pymodules/python2.6/mercurial/dispatch.py", line 16, in run sys.exit(dispatch(sys.argv[1:])) File "/usr/lib/pymodules/python2.6/mercurial/dispatch.py", line 30, in dispatch return _runcatch(u, args) File "/usr/lib/pymodules/python2.6/mercurial/dispatch.py", line 46, in _runcatch return _dispatch(ui, args) File "/usr/lib/pymodules/python2.6/mercurial/dispatch.py", line 454, in _dispatch return runcommand(lui, repo, cmd, fullargs, ui, options, d) File "/usr/lib/pymodules/python2.6/mercurial/dispatch.py", line 324, in runcommand ret = _runcommand(ui, options, cmd, d) File "/usr/lib/pymodules/python2.6/mercurial/dispatch.py", line 505, in _runcommand return checkargs() File "/usr/lib/pymodules/python2.6/mercurial/dispatch.py", line 459, in checkargs return cmdfunc() File "/usr/lib/pymodules/python2.6/mercurial/dispatch.py", line 453, in <lambda> d = lambda: util.checksignature(func)(ui, *args, **cmdoptions) File "/usr/lib/pymodules/python2.6/mercurial/util.py", line 386, in check return func(*args, **kwargs) File "/usr/lib/pymodules/python2.6/mercurial/commands.py", line 1844, in import_ files=files, eolmode=None) File "/usr/lib/pymodules/python2.6/mercurial/patch.py", line 1157, in patch return internalpatch(patchname, ui, strip, cwd, files, eolmode) File "/usr/lib/pymodules/python2.6/mercurial/patch.py", line 1125, in internalpatch ret = applydiff(ui, fp, files, strip=strip, eol=eol) File "/usr/lib/pymodules/python2.6/mercurial/patch.py", line 972, in applydiff for state, values in iterhunks(ui, fp, sourcefile, textmode): File "/usr/lib/pymodules/python2.6/mercurial/patch.py", line 883, in iterhunks current_hunk = binhunk(changed[bfile]) KeyError: 'browser/themes/winstripe/browser/mainwindow-dropdown-arrow.png\r'

Sorry, wrong encoding. It must works.

(In reply to comment #6) > Thunderbird can always copy the old image if they want to. I think the really > relevant question is, should notification.css use the old or the new image? > Also, killing closeSidebar.png in favor of the new image might make sense. Yes. We can use the new close buttons in the sidebar and for the notifications.

Comment on attachment 449609 [details] [diff] [review] patch v3 (In reply to comment #11) > [...] I just removed some spaces between close icons. [...] Looks like you left some lighter pixels in the corners of the second and third icons, which looks unintentional on dark backgrounds. r=me with this explained or fixed. Thanks!

(In reply to comment #17) > Looks like you left some lighter pixels in the corners of the second and third > icons, which looks unintentional on dark backgrounds. r=me with this explained > or fixed. Azat, please comment on this.

Comment on attachment 456172 [details] [diff] [review] patch The icon region is bogus for the selected tab in the non-hover state.

Comment on attachment 458101 [details] [diff] [review] patch alltabs.png has more padding on the bottom than on the top, causing the icon to be misaligned. Please reduce the image height by one pixel and update the #alltabs-button regions in browser.css accordingly. r=me with that

Comment on attachment 458170 [details] [diff] [review] patch thanks!

(In reply to comment #24) > Comment on attachment 458170 [details] [diff] [review] > patch > > thanks! Time for push?

Shouldn't the close button be light red on hover? Current color seems weird and doesn't feel like Windows to me. Usually Windows buttons change color on hover and darken color on pressing, currently it only changes color on pressing.

(In reply to comment #27) > Shouldn't the close button be light red on hover? Current color seems weird and > doesn't feel like Windows to me. Usually Windows buttons change color on hover > and darken color on pressing, currently it only changes color on pressing. Email shorlander@mozilla.com and suggest your idea.

(In reply to comment #28) > (In reply to comment #27) > > Shouldn't the close button be light red on hover? Current color seems weird and > > doesn't feel like Windows to me. Usually Windows buttons change color on hover > > and darken color on pressing, currently it only changes color on pressing. > > Email shorlander@mozilla.com and suggest your idea. No, file a bug!