Jim Jeffery not reading bug-mail 1/2/11 Reporter
	Description • 16 years ago

The 'New tab' button looks like its sitting on a pedestal.  See screenshot here from the Builds Forum:

http://forums.mozillazine.org/viewtopic.php?p=4903505#p4903505

Caused by? https://bugzilla.mozilla.org/show_bug.cgi?id=458487 
or maybe due to the landing of the 'All Tabs Preview' ? 

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b2pre) Gecko/20081105 Minefield/3.1b2pre Firefox/3.0 ID:20081105032350

	Dão Gottwald [:dao]
	Comment 1 • 16 years ago

(In reply to comment #0)
> or maybe due to the landing of the 'All Tabs Preview' ?

nope

	IU
	Comment 2 • 16 years ago

Bug also affects the tab overflow arrows.

	u88484
	Comment 3 • 16 years ago

Why was the whiteboard cleared of things Alex specifically wants added to visual bugs, especially high visibility bugs?  Also, 'polish' from the keyword?

	Dão Gottwald [:dao]
	Comment 4 • 16 years ago

Because this isn't a polish bug. It's probably caused by bug 458487 but in any case it's a core bug.

	Markus Stange [:mstange]
	Comment 5 • 16 years ago

And it might be fixed by bug 456330.

	u88484
	Comment 6 • 16 years ago

(In reply to comment #4)
> Because this isn't a polish bug. It's probably caused by bug 458487 but in any
> case it's a core bug.

Well by Alex's blog post (http://tinyurl.com/56gc6m) it is.  "[polish-hard] - something that is not trival to fix (like rewriting the focus code)"

And yes at first I thought it was an easy fix, as in changing some css but now it is clear it is not 'polish-easy' but 'polish-hard' so it should still have those keywords in the whiteboard.

I'm just trying to get this on Alex's radar so beta 2 dosen't ship with messed up icons...especially on an icon for a "feature" that was just implemented (new tab button on tabbar).

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 7 • 16 years ago

I don't see it on Mac. Could be fixed by patch in bug 456330 or bug 463204.

	Jim Jeffery not reading bug-mail 1/2/11 Reporter
	Comment 8 • 16 years ago

Using an hourly build, changeset:
http://hg.mozilla.org/mozilla-central/rev/18fec592b35b
which should contain the patches in comment #7 did not fix the appearance of the Icons.

	John P Baker
	Comment 9 • 16 years ago

There is something odd with -moz-image-region

.tabs-newtab-button > .toolbarbutton-icon {-moz-image-region: auto}

clears the pedestal for me.

	John P Baker
	Comment 10 • 16 years ago

(In reply to comment #9)
> There is something odd with -moz-image-region
> 
> .tabs-newtab-button > .toolbarbutton-icon {-moz-image-region: auto}
> 
> clears the pedestal for me.

Is it possible that the code in bug 458487 is tiling the image if it is smaller than the -moz-image-region?

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 11 • 16 years ago

Yeah I think we're tiling the image to fill the -moz-image-region. Oops.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 12 • 16 years ago

Work in progress. This should fix the bug. I just need to add some comments and the reftest.

	Ryan VanderMeulen [:RyanVM]
	Comment 15 • 16 years ago

The patch here does fix the icon starting to repeat, but it appears that the icon is still misaligned. It's shifted up relative the height of the adjacent tabs.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 16 • 16 years ago

Hmm, it looked OK to me. Can you modify the testcase in the patch to show the bug?

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 17 • 16 years ago

This is what I see in my build with the patch --- it looks fine to me. Are you seeing something else?

	Mike Beltzner [:beltzner, not reading bugmail]
	Comment 18 • 16 years ago

Highly visible impact on the Windows beta, should fix before b2.

	David Baron :dbaron:
	Comment 19 • 16 years ago

Any chance you could briefly explain the patch?

	David Baron :dbaron:
	Comment 20 • 16 years ago

Comment on attachment 347053 [details] [diff] [review]
fix v1

Well, r+sr=dbaron anyway.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 21 • 16 years ago

If aSourceArea is non-null and references a rectangle that extends outside the image bounds (0,0,imageWidthInAppUnits,imageHeightInAppUnits), we get into a situation where 'fill' extends outside 'dest'. Then the call to DrawImage will trigger tiling, which we don't want.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 22 • 16 years ago

Pushed 635057569226

	Jim Jeffery not reading bug-mail 1/2/11 Reporter
	Comment 23 • 16 years ago

Verified fixed using changeset:

http://hg.mozilla.org/mozilla-central/rev/b5f3b30402cb

Hourly build:

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b2pre) Gecko/20081110 Minefield/3.1b2pre Firefox/3.0 ID:20081110005319

	Alex Faaborg [:faaborg] (Firefox UX)
	Comment 24 • 16 years ago

>"[polish-hard] - something that is not trival to fix (like rewriting the focus
>code)"

Yeah, this is a good example of [polish-hard] [polish-high-visibility], just so everyone is on the same page with the various whiteboard terms we are trying out.

	Dão Gottwald [:dao]
	Comment 25 • 16 years ago

(In reply to comment #24)
> >"[polish-hard] - something that is not trival to fix (like rewriting the focus
> >code)"
> 
> Yeah, this is a good example of [polish-hard] [polish-high-visibility], just so
> everyone is on the same page with the various whiteboard terms we are trying
> out.

I assume that status whiteboard messages like [polish-hard] only nail the 'polish' keyword down, but don't fundamentally change its meaning. This bug doesn't meet the criteria for 'polish' (https://bugzilla.mozilla.org/describekeywords.cgi), because it isn't about the UI -- it's a Core:Layout regression that can manifest itself anywhere.