348319 - Some inactive toolbar buttons (back button) are highlighted on mouse click

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Arie Paap [:wildmyron]]

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

When some of the toolbar buttons are inactive, clicking on them will cause them to highlight. I have noticed this with the back and forward dropdown buttons, the refresh button, the copy button and the paste button. This doesn't happen with the stop, report site and cut buttons. This is related to the visual refresh for 2.0.

I couldn't find any other buttons which would have an inactive state.

Screenshot to follow.

Build ID: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060810 BonEcho/2.0b1 ID:2006081003

Reproducible: Always

Comment 1

[Arie Paap [:wildmyron]]

Attachment: Highlighted inactive button

Screenshot showing affect when clicking on back button

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Attachment: make sure disabled styles take precedence (winstripe)

Not sure if this affects pinstripe too, Adam is testing.

Comment 3

[Adam Guthrie]

Attachment: make sure disabled styles take precedence (pinstripe)

Not as much seems to be affected on Mac, but I probably missed something...

Comment 4

[:Gavin Sharp [email: [e-mail]]]

*** Bug 348572 has been marked as a duplicate of this bug. ***

Comment 5

[Asaf Romano (gone)]

I would rather do :not([disabled="true"] everywhere :active is used.

Comment 6

[Adam Guthrie]

*** Bug 348658 has been marked as a duplicate of this bug. ***

Comment 7

[:Gavin Sharp [email: [e-mail]]]

*** Bug 350108 has been marked as a duplicate of this bug. ***

Comment 8

[Ryan Flint [:rflint] (ping via IRC for reviews)]

*** Bug 350598 has been marked as a duplicate of this bug. ***

Comment 9

[Reed Loden [:reed]]

This is a DUPE of the new bug 350590. Please figure out the best fix of the two patches (or take both).

Comment 10

[Reed Loden [:reed]]

(In reply to comment #9)
> This is a DUPE of the new bug 350590. Please figure out the best fix of the two
> patches (or take both).

Ignore me. I was looking at this screenshot twice (thinking it was for the other bug, too). :(

Comment 11

[Phil Ringnalda (:philor)]

*** Bug 350794 has been marked as a duplicate of this bug. ***

Comment 12

[Simon Bünzli]

*** Bug 350853 has been marked as a duplicate of this bug. ***

Comment 13

[Mike Connor [:mconnor]]

not a blocker, would take patch (someone poke me in 4-5 hours and I'll take a look)

Comment 14

[Mike Beltzner [:beltzner, not reading bugmail]]

Uh, this so totally does block.

Comment 15

[Ria Klaassen (not reading all bugmail)]

*** Bug 351161 has been marked as a duplicate of this bug. ***

Comment 16

[Nick Thomas [:nthomas] (UTC+12)]

*** Bug 351187 has been marked as a duplicate of this bug. ***

Comment 17

[Mike Connor [:mconnor]]

Comment on attachment 233248 [details] [diff] [review]
make sure disabled styles take precedence (winstripe)

r+a=me, straightforward belt-and-suspenders patch

Comment 18

[Mike Connor [:mconnor]]

Comment on attachment 233280 [details] [diff] [review]
make sure disabled styles take precedence (pinstripe)

r+a=me, thanks Adam!

Comment 19

[:Gavin Sharp [email: [e-mail]]]

mozilla/browser/themes/pinstripe/browser/browser.css 	1.11.4.40
mozilla/browser/themes/winstripe/browser/browser.css 	1.17.2.52

Comment 20

[Reed Loden [:reed]]

*** Bug 351257 has been marked as a duplicate of this bug. ***

Comment 21

[Adam Guthrie]

I would appreciate it if someone would verify this is fixed on Mac for all icon sizes and combinations. This involves adding all the icons to the toolbar and clicking on then in their inactive state. (Note that not all icons will have an inactive state; e.g. the bookmarks button.)

Comment 22

[Ed Hume]

In reply to comment #5: there is an alternative to both putting !important on every [disabled=true] pseudo-class and putting :not([disabled="true"] everywhere :active is used.

With every use of [disabled=true], pair it with a [disabled=true]:hover:active line:


#any-button[disabled=true]:hover:active,
#any-button[disabled=true] { etc. }

Using the !important marker in a default theme means trouble when users try to modify their browsers with userChrome.css files.

Comment 23

[Cheng Khoon]

FYI the inactive back and forward button in Help window still exhibits this problem. Should a new bug be logged for it or should it be tracked in this bug?

Comment 24

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #22)
> Using the !important marker in a default theme means trouble when users try to
> modify their browsers with userChrome.css files.

You're right - could you file a new bug for this, and attach a patch if possible?

(In reply to comment #23)
> FYI the inactive back and forward button in Help window still exhibits this
> problem. Should a new bug be logged for it or should it be tracked in this bug?

New bug, please.

Comment 25

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #24)
> (In reply to comment #22)
> > Using the !important marker in a default theme means trouble when users try to
> > modify their browsers with userChrome.css files.
> 
> You're right - could you file a new bug for this, and attach a patch if
> possible?

I'll fix this here. Mano and I have concluded that using Mano's suggestion from comment 5 is the best choice.

> (In reply to comment #23)
> > FYI the inactive back and forward button in Help window still exhibits this
> > problem. Should a new bug be logged for it or should it be tracked in this bug?
> 
> New bug, please.

This was filed as bug 351518.

Comment 26

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #25)
> (In reply to comment #24)
> > (In reply to comment #22)
> > > Using the !important marker in a default theme means trouble when users try to
> > > modify their browsers with userChrome.css files.
> > 
> > You're right - could you file a new bug for this, and attach a patch if
> > possible?
> 
> I'll fix this here. Mano and I have concluded that using Mano's suggestion from
> comment 5 is the best choice.

On second thought, I'll fix it in bug 348319.

Comment 27

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #26)
> On second thought, I'll fix it in bug 348319.

I meant bug 351855, of course.

Comment 28

[Tracy Walker [:tracy]]

verified with 2.0.0.4 on Mac and linux