Closed Bug 463217 Opened 11 years ago Closed 11 years ago

New Tab and Overflow Scroll buttons look like they're sitting on a pedestal

Categories

(Core :: Layout: Images, Video, and HTML Frames, defect, P2)

defect

Tracking

()

VERIFIED FIXED
mozilla1.9.1b2

People

(Reporter: jmjjeffery, Assigned: roc)

References

Details

(Keywords: regression, verified1.9.1)

Attachments

(2 files, 1 obsolete file)

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
OS: Windows Vista → All
Summary: New Tab Button looks like its setting on a pedestal → New Tab Button looks like it's setting on a pedestal
Flags: blocking1.9.1?
(In reply to comment #0)
> or maybe due to the landing of the 'All Tabs Preview' ?

nope
Bug also affects the tab overflow arrows.
Keywords: polish
Summary: New Tab Button looks like it's setting on a pedestal → New Tab and Overflow Scroll buttons look like they're sitting on a pedestal
Whiteboard: [polish-easy][polish-visual][polish-high-visibility]
Keywords: polish
Whiteboard: [polish-easy][polish-visual][polish-high-visibility]
Why was the whiteboard cleared of things Alex specifically wants added to visual bugs, especially high visibility bugs?  Also, 'polish' from the keyword?
Because this isn't a polish bug. It's probably caused by bug 458487 but in any case it's a core bug.
And it might be fixed by bug 456330.
(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).
I don't see it on Mac. Could be fixed by patch in bug 456330 or bug 463204.
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.
There is something odd with -moz-image-region

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

clears the pedestal for me.
Assignee: nobody → roc
(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?
Yeah I think we're tiling the image to fill the -moz-image-region. Oops.
Attached patch fix (obsolete) — Splinter Review
Work in progress. This should fix the bug. I just need to add some comments and the reftest.
Attached patch fix v1Splinter Review
Attachment #347051 - Attachment is obsolete: true
Attachment #347053 - Flags: superreview?(dbaron)
Attachment #347053 - Flags: review?(dbaron)
Duplicate of this bug: 463666
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.
Hmm, it looked OK to me. Can you modify the testcase in the patch to show the bug?
Attached image screenshot
This is what I see in my build with the patch --- it looks fine to me. Are you seeing something else?
Highly visible impact on the Windows beta, should fix before b2.
Flags: blocking1.9.1? → blocking1.9.1+
Any chance you could briefly explain the patch?
Attachment #347053 - Flags: superreview?(dbaron)
Attachment #347053 - Flags: superreview+
Attachment #347053 - Flags: review?(dbaron)
Attachment #347053 - Flags: review+
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.
Pushed 635057569226
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Flags: in-testsuite+
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
Status: RESOLVED → VERIFIED
>"[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.
(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.
Hardware: PC → All
Product: Core → Core Graveyard
Product: Core Graveyard → Core
You need to log in before you can comment on or make changes to this bug.