285971 - [MASv1.8a6+, Classic] Back/Forward buttons still respond to :active while disabled

Status: VERIFIED FIXED

(SeaMonkey :: Themes, defect)

Description

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050312] (nightly) (W98SE)

1. Mouse press theses 2 buttons while they are disabled (no history)

The text (I'm in text-only mode) moves as it does when the button is not disabled.

I'm Classic, maybe it's the same for other themes !?

This does not happen when Stop button, or others...
It may be related to this 2 buttons having an associated dropdown menu !?

Comment 1

[[e-mail]]

Weird, I see this in my Windows build but not in my Linux build...

Comment 2

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7.5) Gecko/20041217] (release) (W98SE)

No bug.

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b) Gecko/20050217] (release) (W98SE)

Same bug.

I'll try to narrow the timeframe;
and test whether this was caused by our conversion to |:hover:active| syntax...

Comment 3

[Serge Gautherie (:sgautherie)]

(In reply to comment #2)
> I'll try to narrow the timeframe;
> and test whether this was caused by our conversion to |:hover:active| syntax...

It's the same with |[button___="true"]| syntax :-|
I'll have to test older builds...

The bug is the same in the 3 modes: text, images, both.

Comment 4

[Serge Gautherie (:sgautherie)]

(In reply to comment #3)
> (In reply to comment #2)
> > I'll try to narrow the timeframe;
> > and test whether this was caused by our conversion to |:hover:active| syntax...
> 
> It's the same with |[button___="true"]| syntax :-|
> I'll have to test older builds...

I previously reverted the 2 |forward-button| rules;
But the timeframe corresponds to our 20041212 check-in...

Good:
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a1) Gecko/20040520
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a5) Gecko/20041122
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a6) Gecko/20041209

Bad:
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a6) Gecko/20041217
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a6) Gecko/20050111

Odd, I try to revert more patched rules... (later)

Comment 5

[[e-mail]]

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

Comment 6

[Stefan [:stefanh]]

It looks related to the conversion to hover:active, replacing hover:active with
[buttondown="true"] in global/toolbarbutton.css and communicator/button.css
fixes the issue (changes in global/toolbarbutton.css fixes the mac-specific issue).

Comment 7

[Stefan [:stefanh]]

On mac there's also an issue when pressing the dropmarker on active back/fwd
buttons: The whole button get the dark background-color (not just the dropmarker
area) and it moves. This looks also related to the :hover:active conversion.

Comment 8

[Serge Gautherie (:sgautherie)]

(In reply to comment #5)
> *** Bug 286020 has been marked as a duplicate of this bug. ***

Quote
{{
It appears that build 2004121106 (mac) is working fine, but mac builds from
2004121306 and on has the issue.
}}

Then:
*Mac and Windows; but not Linux.
*timeframe narrowed further.

Comment 9

[Serge Gautherie (:sgautherie)]

(In reply to comment #6)
> It looks related to the conversion to hover:active, replacing hover:active with
> [buttondown="true"] in global/toolbarbutton.css and communicator/button.css
> fixes the issue (changes in global/toolbarbutton.css fixes the mac-specific
issue).

Ah ... I had tried reverting classic/navigator.css forward button, which did not
help.

On Windows, clicking on inactive dropmarker shows the current bug too...

Neil, any hint ?

(In reply to comment #7)
> On mac there's also an issue when pressing the dropmarker on active back/fwd
> buttons: The whole button get the dark background-color (not just the dropmarker
> area) and it moves. This looks also related to the :hover:active conversion.

I think this is the "new" intended behaviour !? (see bug 257280)

Comment 10

[Stefan [:stefanh]]

> (In reply to comment #7)
> > On mac there's also an issue when pressing the dropmarker on active back/fwd
> > buttons: The whole button get the dark background-color (not just the dropmarker
> > area) and it moves. This looks also related to the :hover:active conversion.
> 
> I think this is the "new" intended behaviour !? (see bug 257280)

From bug 257280, comment #5:
"I also checked interacting with the dropdown (right) part of the button:
without the patch, the image part always stays normal (only its focus
activates);
with the patch, the image state dynamically responds to the same state as the
dropdown.
I let you decide if this is a regression or an enhancement !"

I would say it's a regression... However, I'm not the man in charge ;)

All these issues can be fixed by changing the css a bit. I'll attach a fix for
the first issue so you can try it on your system.

Comment 11

[Stefan [:stefanh]]

Attachment: Fix disabled/enabled hover:active in button.css

OK, this will fix the reported issue. I'll fix the mac issue with the
.toolbarbutton-menubutton-button dark background-color in a while. This patch
will also fix the movement of the button when pressing the dropmarker. The
background-color is still dark, though. I'll have to look into this further...,
will post another patch later.

Comment 12

[Stefan [:stefanh]]

Comment on attachment 177365 [details] [diff] [review]
Fix disabled/enabled hover:active in button.css

Hmm, this patch has the disadvantage that the back/fwd etc buttons doesn't move
when you press them... The style rules are quite complicated, styles in
communicator/button.css are interacting with styles in
global/mac,win,unix/toolbarbutton.css and I haven't got any windows/unix
machine to test on.

Comment 13

[Serge Gautherie (:sgautherie)]

(In reply to comment #10)
> > (In reply to comment #7)
> > I think this is the "new" intended behaviour !? (see bug 257280)
> 
> I would say it's a regression... However, I'm not the man in charge ;)

It seems it's not, see bug 257280 comment 8:
please, double-check with Neil and/or Michael before proceeding on that point.

Comment 14

[Serge Gautherie (:sgautherie)]

Comment on attachment 177365 [details] [diff] [review]
Fix disabled/enabled hover:active in button.css

>Index: communicator/button.css
>===================================================================
>RCS file: /cvsroot/mozilla/themes/classic/communicator/button.css,v
>retrieving revision 1.11
>diff -U10 -r1.11 button.css
>--- communicator/button.css	15 Dec 2004 09:40:02 -0000	1.11
>+++ communicator/button.css	14 Mar 2005 14:34:43 -0000
>@@ -48,27 +48,27 @@
> .toolbarbutton-1,
> .toolbarbutton-1 > .toolbarbutton-menubutton-button,
>-.toolbarbutton-1[disabled="true"]:hover:active {
>+.toolbarbutton-1[disabled="true"]:hover:active,
>+.toolbarbutton-1[disabled="true"]:hover:active > .toolbarbutton-menubutton-button {
>   padding: 2px 7px 2px 6px;
> }
> 
> .toolbarbutton-1:hover:active,
>-.toolbarbutton-1[open="true"],
>-.toolbarbutton-1:hover:active > .toolbarbutton-menubutton-button {
>+.toolbarbutton-1[open="true"] {
>   padding: 3px 6px 1px 7px;
> }

What happens if you leave the latter line unchanged, simply adding the
|[disabled="true"]| line ?

Comment 15

[Stefan [:stefanh]]

Attachment: Fix for :hover:active on disabled buttons [Check in: See comment 22]

OK, this will only fix the disabled buttons response to hover:active. The
change in global/mac/toolbarbutton.css fixes the mac-specific issue (dark grey
background on hover:active .toolbarbutton-menubutton-button when buttons are
disabled).

However, I feel that this is a sort of a hack... suggestions welcome ;)

Comment 16

[Stefan [:stefanh]]

Comment on attachment 177390 [details] [diff] [review]
Fix for :hover:active on disabled buttons
[Check in: See comment 22]

Neil, can you take a look at this? It's not really that beautiful, but it
works...

Comment 17

[Serge Gautherie (:sgautherie)]

(In reply to comment #15)
> Created an attachment (id=177390) [edit]
> Fix for :hover:active on disabled buttons
> 
> OK, this will only fix the disabled buttons response to hover:active.

Did you check Modern theme too ? (I'm not sure if its code misbehaves or not)

By extension, Qute has a <communicator/button.css> too, and its code seems to
have the same bug ... separate patch.


[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.6) Gecko/20050223
Firefox/1.0.1]

FFv1.0.x works fine, as MASv1.7.x.

Yet, it would be interesting to check FireFox vTrunk too...
Maybe unaffected since it uses <global/button.css> only !?

Comment 18

[Serge Gautherie (:sgautherie)]

(In reply to comment #17)
> Did you check Modern theme too ? (I'm not sure if its code misbehaves or not)

Modern theme is free from this bug, as I guessed by its code.

Comment 19

[Serge Gautherie (:sgautherie)]

(In reply to comment #17)
> By extension, Qute has a <communicator/button.css> too, and its code seems to
> have the same bug ... separate patch.

I'll fix it in the other bug, since this file patch hasn't been checked-in yet.

> Yet, it would be interesting to check FireFox vTrunk too...

After looking back at the checked-in patches, I think we can skip that check.

Comment 20

[Serge Gautherie (:sgautherie)]

(In reply to comment #16)
> (From update of attachment 177390 [details] [diff] [review] [edit])
> Neil, can you take a look at this? It's not really that beautiful, but it
> works...

Unless there is something wrong in the |:hover:active| handling code, compared
to |[buttondown="true"]|,
that would seem to be the right fix.
(I wanted to try some other syntaxes, which did not work.)

Yet, I have no clue on why Linux did not misbehave :-/

Comment 21

[[e-mail]]

Comment on attachment 177390 [details] [diff] [review]
Fix for :hover:active on disabled buttons
[Check in: See comment 22]

r+sr=me for the communicator changes, but not the mac changes, as they're not
consistent with the rest of the file.

Comment 22

[[e-mail]]

I checked in the communicator portion of the patch.

Comment 23

[Serge Gautherie (:sgautherie)]

(In reply to comment #21)
> (From update of attachment 177390 [details] [diff] [review] [edit])
> r+sr=me for the communicator changes, but not the mac changes, as they're not
> consistent with the rest of the file.

Neil, for the record, can you set the r+/sr+ flags too ?

Stefan, I didn't pay attention ... I guess that you should add the 2 new rules after
{{
138 toolbarbutton[type="menu-button"][disabled="true"]:hover:active {
}}

Comment 24

[Stefan [:stefanh]]

Attachment: Mac styles in correct place

Ah, yes. Now I understand. I guess I was a bit distracted before ;)

Neil, If you want this in I can fix the other mac stuff separately (that we
talked about on irc) in another bug later on. Would be nice to close this one
imho. Otherwise, just ignore/cancel my requests.

Comment 25

[Serge Gautherie (:sgautherie)]

(In reply to comment #24)
> Neil, If you want this in I can fix the other mac stuff separately (that we
> talked about on irc) in another bug later on. Would be nice to close this one
> imho. Otherwise, just ignore/cancel my requests.

Yes:
if they are other regressions from bug 257280, add them here;
otherwise, file a separate bug ;-)

Comment 26

[[e-mail]]

Comment on attachment 177524 [details] [diff] [review]
Mac styles in correct place

Sorry for any misunderstanding but I think this is worse than the previous
patch.

Comment 27

[[e-mail]]

I actually meant to modify .toolbarbutton-menubutton-button[disabled="true"]

Comment 28

[Stefan [:stefanh]]

Attachment: "background-color: transparent !important;" instead [Checked in: Comment 30]

Agh, sorry. I suppose you ment like this?

Comment 29

[Asaf Romano (gone)]

Comment on attachment 177569 [details] [diff] [review]
"background-color: transparent !important;" instead
[Checked in: Comment 30]

works fine

Comment 30

[Asaf Romano (gone)]

Comment on attachment 177569 [details] [diff] [review]
"background-color: transparent !important;" instead
[Checked in: Comment 30]

checked in by Neil.

Comment 31

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050318] (nightly) (W98SE)

V.Fixed

Comment 32

[Serge Gautherie (:sgautherie)]

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

Comment 33

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050502] (nightly) (W98SE)

This regressed (again) :-(

Probably after
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050329] (nightly) (W98SE)
or even
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050420] (nightly) (W98SE)
...

Comment 34

[Stefan [:stefanh]]

Hmm, I don't see this on my 2005050111 mac build.

Comment 35

[Serge Gautherie (:sgautherie)]

(In reply to comment #34)
> Hmm, I don't see this on my 2005050111 mac build.

My apologies.

I checked again, no bug: I must have been looking at my v1.8b1 :-<