Closed Bug 349196 Opened 14 years ago Closed 13 years ago

All Tabs menu doesn't use right edge image with browser.tabs.closeButtons = 3

Categories

(Firefox :: Tabbed Browser, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 3 alpha8

People

(Reporter: hsumen, Assigned: dao)

References

Details

(Keywords: polish, Whiteboard: [Fx2 theme change])

Attachments

(4 files, 11 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060817 BonEcho/2.0b1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060817 BonEcho/2.0b1

alltabs-box-start-bkgnd.png seems to be getting chopped of on Windows XP using either the Luna or Classic OS themes.

Reproducible: Always

Steps to Reproduce:
1. Open a few tabs.
2. View the All Tabs button.
3.

Actual Results:  
The button appears to be chopped off.

Expected Results:  
The button should have a rounded right-corner and a clearly defined border.
Can you give a screenshot of this?  Specifically, I'm wondering if this is for the "tab close button to right of all tabs menu" case.
Attached image alltabs (obsolete) —
Attached image alltabs with tab overflow (obsolete) —
Sorry, that first pic isn't cropped right, but the other two should show it. In global/icons of classic.jar there is the following image which doesn't seem to be getting displayed...
Attached image alltabs-right (obsolete) —
Status: UNCONFIRMED → NEW
Ever confirmed: true
OK, so looking at your screenshots, this is indeed for the "closeButtons = 3" case.

Changing summary in hopes of being clearer, requesting blocking.
Flags: blocking-firefox2?
Summary: alltabs-box-start-bkgnd.png is not being displayed → All Tabs menu doesn't use right edge image with browser.tabs.closeButtons = 3
Target Milestone: --- → Firefox 2
Flags: blocking-firefox2? → blocking-firefox2-
Keywords: polish
Whiteboard: [Fx2 theme change][would take patch]
*** Bug 352356 has been marked as a duplicate of this bug. ***
Dao: Could you also have a look at this, once you're done with bug 352321?
One solution would be to add a right border to the alltabs button, which gets cropped off when the button is the :last-child.

(In reply to comment #6)
> Created an attachment (id=234487) [edit]
> alltabs-right

That's for RTL mode, I guess.
(In reply to comment #10)
Why not just use tab-middle-bkgnd.png as a background for the close button's container (and maybe add a 1px separator to the close button's container)? This should be a cheap fix which makes the close button look like part of the whole tab bar concept by only modifying the close button0s appearance itself (less regression risks).
(In reply to comment #11)
Yes, that would work. Images coming.
Update to browser/themes/winstripe/browser/browser.css:

.tabs-closebutton {
  list-style-image: url("chrome://global/skin/icons/close.png");
  -moz-appearance: none;
  -moz-image-region: rect(0px, 16px, 16px, 0px);
  padding: 4px 2px 3px 2px;
  margin-bottom: 1px;
  border: none !important;
  background: -moz-dialog url("chrome://global/skin/icons/close-bkgnd.png");
}

.tabs-closebutton:hover {
  -moz-image-region: rect(0px, 32px, 16px, 16px);
  background-image: url("chrome://global/skin/icons/close-bkgnd-hover.png");
}
Attached image close-bkgnd.png (obsolete) —
Attachment #238052 - Flags: review?
Attached image close-bkgnd-hover.png (obsolete) —
Attachment #238053 - Flags: review?
(In reply to comment #12)
>   padding: 4px 2px 3px 2px;

better:

>   padding: 4px 2px 3px;

Now somebody would have to create the patch.
Seth? :)
Attached image screenshot (obsolete) —
Attached image Sidebar misapply (obsolete) —
Unless I'm mistaken the CSS as it is now will also affect the Sidebar close buttons as in the attachment.  Adding a parent selector (.tabs-closebutton-box > ) to the rules seems to fix the problem (and is how I did it in my Stylish fix 
http://userstyles.org/style/show/1063).  I think this would make the stylesheet something like:

.tabs-closebutton {
  list-style-image: url("chrome://global/skin/icons/close.png");
  -moz-appearance: none;
  -moz-image-region: rect(0px, 16px, 16px, 0px);
  padding: 4px 2px 3px 2px;
  margin-bottom: 1px;
  border: none !important;
}

.tabs-closebutton-box > .tabs-closebutton {
  background: -moz-dialog url("chrome://global/skin/icons/close-bkgnd.png");
}

.tabs-closebutton:hover {
  -moz-image-region: rect(0px, 32px, 16px, 16px);
}

.tabs-closebutton-box > .tabs-closebutton:hover {
  background-image: url("chrome://global/skin/icons/close-bkgnd-hover.png");
}
(In reply to comment #17)
> Unless I'm mistaken the CSS as it is now will also affect the Sidebar close
> buttons as in the attachment.

You're right. I don't understand why the class is named .tabs-closebutton, though. That's misleading.
*** Bug 357165 has been marked as a duplicate of this bug. ***
*** Bug 357698 has been marked as a duplicate of this bug. ***
Flags: blocking1.8.1.1?
Attachment #238052 - Flags: review? → ui-review?(beltzner)
Attachment #238053 - Flags: review? → ui-review?(beltzner)
Polish is nice, but not blocking 1.8.1.1: moving to the "wanted" list.
Flags: wanted1.8.1.x+
Flags: blocking1.8.1.1?
Flags: blocking1.8.1.1-
Flags: wanted1.8.1.x+
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → dao
Attachment #238052 - Attachment is obsolete: true
Attachment #238053 - Attachment is obsolete: true
Attachment #238204 - Attachment is obsolete: true
Attachment #240933 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #238052 - Flags: ui-review?(beltzner)
Attachment #238053 - Flags: ui-review?(beltzner)
Attached image close-bkgnd.png
Attachment #234482 - Attachment is obsolete: true
Attachment #234483 - Attachment is obsolete: true
Attachment #234485 - Attachment is obsolete: true
Attachment #234487 - Attachment is obsolete: true
Attached image screenshot
Attachment #260502 - Flags: ui-review?(beltzner)
Comment on attachment 260500 [details] [diff] [review]
patch

I'll attach a RTL compatible patch later
Target Milestone: Firefox 2 → ---
Attached image close-bkgnd-rtl.png
Attached patch patch (part 1) (obsolete) — Splinter Review
Attachment #260500 - Attachment is obsolete: true
Attachment #260568 - Flags: review?(robert.bugzilla)
Flags: blocking-firefox3?
Target Milestone: --- → Firefox 3 beta1
Flags: blocking-firefox3? → blocking-firefox3-
Whiteboard: [Fx2 theme change][would take patch] → [Fx2 theme change][wanted-firefox3]
Comment on attachment 260568 [details] [diff] [review]
patch (part 1)

You need to add these files to the jar.mn as well.
Attachment #260568 - Flags: review?(robert.bugzilla) → review-
Attached patch patch (part 2) (obsolete) — Splinter Review
Attachment #271990 - Flags: review?(robert.bugzilla)
Attachment #260568 - Attachment description: patch → patch (part 1)
Attachment #260568 - Flags: review- → review?(robert.bugzilla)
Attachment #260568 - Flags: review?(robert.bugzilla) → review+
Attachment #271990 - Flags: review?(robert.bugzilla) → review+
Target Milestone: Firefox 3 M7 → Firefox 3 M8
Attachment #260502 - Flags: ui-review?(beltzner) → ui-review+
Keywords: checkin-needed
would somebody please check this in?
Attachment #260568 - Attachment is obsolete: true
Attachment #271990 - Attachment is obsolete: true
Checking in browser/base/content/tabbrowser.xml;
/cvsroot/mozilla/browser/base/content/tabbrowser.xml,v  <--  tabbrowser.xml
new revision: 1.239; previous revision: 1.238
done
Checking in browser/themes/winstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/winstripe/browser/browser.css,v  <--  browser.css
new revision: 1.77; previous revision: 1.76
done
Checking in browser/themes/winstripe/browser/jar.mn;
/cvsroot/mozilla/browser/themes/winstripe/browser/jar.mn,v  <--  jar.mn
new revision: 1.42; previous revision: 1.41
done
Checking in browser/themes/winstripe/browser/tabbrowser/tabbrowserBindings.xml;
/cvsroot/mozilla/browser/themes/winstripe/browser/tabbrowser/tabbrowserBindings.xml,v  <--  tabbrowserBindings.xml
new revision: 1.13; previous revision: 1.12
done
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Missed image files

RCS file: /cvsroot/mozilla/browser/themes/winstripe/browser/tabbrowser/close-bkgnd-rtl.png,v
done
Checking in close-bkgnd-rtl.png;
/cvsroot/mozilla/browser/themes/winstripe/browser/tabbrowser/close-bkgnd-rtl.png,v  <--  close-bkgnd-rtl.png
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/browser/themes/winstripe/browser/tabbrowser/close-bkgnd.png,v
done
Checking in close-bkgnd.png;
/cvsroot/mozilla/browser/themes/winstripe/browser/tabbrowser/close-bkgnd.png,v  <--  close-bkgnd.png
initial revision: 1.1
done
Flags: wanted-firefox3+
Whiteboard: [Fx2 theme change][wanted-firefox3] → [Fx2 theme change]
You need to log in before you can comment on or make changes to this bug.