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

RESOLVED FIXED in Firefox 3 alpha8

Status

()

Firefox
Tabbed Browser
RESOLVED FIXED
11 years ago
3 years ago

People

(Reporter: hhh, Assigned: dao)

Tracking

({polish})

unspecified
Firefox 3 alpha8
x86
Windows XP
polish
Points:
---
Bug Flags:
blocking-firefox3 -
wanted-firefox3 +
blocking1.8.1.1 -
blocking-firefox2 -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Fx2 theme change])

Attachments

(4 attachments, 11 obsolete attachments)

(Reporter)

Description

11 years ago
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.

Comment 1

11 years ago
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.
(Reporter)

Comment 2

11 years ago
Created attachment 234482 [details]
alltabs
(Reporter)

Comment 3

11 years ago
Created attachment 234483 [details]
alltabs with tab overflow
(Reporter)

Comment 4

11 years ago
Created attachment 234485 [details]
alltabs:hover w/ browser.tabs.closeButtons @ 3
(Reporter)

Comment 5

11 years ago
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...
(Reporter)

Comment 6

11 years ago
Created attachment 234487 [details]
alltabs-right

Updated

11 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Blocks: 347454

Comment 7

11 years ago
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]

Comment 8

11 years ago
*** Bug 352356 has been marked as a duplicate of this bug. ***

Comment 9

11 years ago
Dao: Could you also have a look at this, once you're done with bug 352321?
(Assignee)

Comment 10

11 years ago
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.

Comment 11

11 years ago
(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).
(Assignee)

Comment 12

11 years ago
(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");
}
(Assignee)

Comment 13

11 years ago
Created attachment 238052 [details]
close-bkgnd.png
Attachment #238052 - Flags: review?
(Assignee)

Comment 14

11 years ago
Created attachment 238053 [details]
close-bkgnd-hover.png
Attachment #238053 - Flags: review?
(Assignee)

Comment 15

11 years ago
(In reply to comment #12)
>   padding: 4px 2px 3px 2px;

better:

>   padding: 4px 2px 3px;

Now somebody would have to create the patch.
Seth? :)
(Assignee)

Comment 16

11 years ago
Created attachment 238204 [details]
screenshot

Comment 17

11 years ago
Created attachment 240933 [details]
Sidebar misapply

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");
}
(Assignee)

Comment 18

11 years ago
(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.

Comment 19

11 years ago
*** Bug 357165 has been marked as a duplicate of this bug. ***
*** Bug 357698 has been marked as a duplicate of this bug. ***

Updated

11 years ago
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+
(Assignee)

Comment 22

10 years ago
Created attachment 260500 [details] [diff] [review]
patch
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)
(Assignee)

Comment 23

10 years ago
Created attachment 260501 [details]
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
(Assignee)

Comment 24

10 years ago
Created attachment 260502 [details]
screenshot
Attachment #260502 - Flags: ui-review?(beltzner)
(Assignee)

Comment 25

10 years ago
Comment on attachment 260500 [details] [diff] [review]
patch

I'll attach a RTL compatible patch later
(Assignee)

Updated

10 years ago
Target Milestone: Firefox 2 → ---
(Assignee)

Comment 26

10 years ago
Created attachment 260566 [details]
close-bkgnd-rtl.png
(Assignee)

Comment 27

10 years ago
Created attachment 260568 [details] [diff] [review]
patch (part 1)
Attachment #260500 - Attachment is obsolete: true
(Assignee)

Updated

10 years ago
Attachment #260568 - Flags: review?(robert.bugzilla)
(Assignee)

Updated

10 years ago
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-
(Assignee)

Comment 29

10 years ago
Created attachment 271990 [details] [diff] [review]
patch (part 2)
Attachment #271990 - Flags: review?(robert.bugzilla)
(Assignee)

Updated

10 years ago
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+
(Assignee)

Updated

10 years ago
Target Milestone: Firefox 3 M7 → Firefox 3 M8
Attachment #260502 - Flags: ui-review?(beltzner) → ui-review+
(Assignee)

Updated

10 years ago
Keywords: checkin-needed
(Assignee)

Comment 30

10 years ago
Created attachment 277873 [details] [diff] [review]
synced with trunk

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
Last Resolved: 10 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.