Closed Bug 285971 Opened 20 years ago Closed 19 years ago

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

Categories

(SeaMonkey :: Themes, defect)

defect
Not set
minor

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.8beta2

People

(Reporter: sgautherie, Assigned: stefanh)

References

Details

(Keywords: classic, regression)

Attachments

(2 files, 2 obsolete files)

[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 !?
Weird, I see this in my Windows build but not in my Linux build...
[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...
Keywords: regression
(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.
Summary: Back/Forward buttons still respond to :active while disabled → (MASv1.8xx+) Back/Forward buttons still respond to :active while disabled
(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)
Summary: (MASv1.8xx+) Back/Forward buttons still respond to :active while disabled → (MASv1.8a6+) Back/Forward buttons still respond to :active while disabled
*** Bug 286020 has been marked as a duplicate of this bug. ***
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).
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.
(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.
Blocks: 257280
OS: Windows 98 → All
Hardware: PC → All
(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)
> (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.
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 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.
(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 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 ?
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 ;)
Attachment #177365 - Attachment is obsolete: true
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...
Attachment #177390 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #177390 - Flags: review?(neil.parkwaycc.co.uk)
(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 !?
(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.
Keywords: classic
Summary: (MASv1.8a6+) Back/Forward buttons still respond to :active while disabled → (MASv1.8a6+, Classic) Back/Forward buttons still respond to :active while disabled
(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.
(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 :-/
Assignee: themes → stefan_h
Flags: blocking1.8b2?
Target Milestone: --- → mozilla1.8beta2
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.
I checked in the communicator portion of the patch.
Attachment #177390 - Attachment description: Fix for :hover:active on disabled buttons → Fix for :hover:active on disabled buttons [Check in: See comment 22]
Attachment #177390 - Attachment is obsolete: true
(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 {
}}
Attached patch Mac styles in correct place (obsolete) — Splinter Review
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.
Attachment #177524 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #177524 - Flags: review?(neil.parkwaycc.co.uk)
(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 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.
Attachment #177524 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #177524 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #177524 - Flags: review-
I actually meant to modify .toolbarbutton-menubutton-button[disabled="true"]
Agh, sorry. I suppose you ment like this?
Attachment #177524 - Attachment is obsolete: true
Attachment #177569 - Flags: review?(bugs.mano)
Comment on attachment 177569 [details] [diff] [review]
"background-color: transparent !important;" instead
[Checked in: Comment 30]

works fine
Attachment #177569 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #177569 - Flags: review?(bugs.mano)
Attachment #177569 - Flags: review+
Attachment #177569 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
Comment on attachment 177569 [details] [diff] [review]
"background-color: transparent !important;" instead
[Checked in: Comment 30]

checked in by Neil.
Attachment #177569 - Attachment description: New version, "background-color: transparent !important;" instead → [checked in] "background-color: transparent !important;" instead
Attachment #177569 - Attachment description: [checked in] "background-color: transparent !important;" instead → "background-color: transparent !important;" instead [Checked in: Comment 30]
Attachment #177569 - Attachment is obsolete: true
Status: NEW → RESOLVED
Closed: 20 years ago
Flags: blocking1.8b2?
Resolution: --- → FIXED
Summary: (MASv1.8a6+, Classic) Back/Forward buttons still respond to :active while disabled → [MASv1.8a6+, Classic] Back/Forward buttons still respond to :active while disabled
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050318] (nightly) (W98SE)

V.Fixed
Status: RESOLVED → VERIFIED
*** Bug 287925 has been marked as a duplicate of this bug. ***
[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)
...
Status: VERIFIED → REOPENED
Flags: blocking1.8b2?
Resolution: FIXED → ---
Hmm, I don't see this on my 2005050111 mac build.
(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 :-<
Status: REOPENED → RESOLVED
Closed: 20 years ago19 years ago
Flags: blocking1.8b2?
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
Attachment #177390 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #177390 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #177390 - Attachment is obsolete: false
Attachment #177569 - Attachment is obsolete: false
Product: Core → SeaMonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: