120527 - When another button has focus, default button is still marked as default

Status: RESOLVED FIXED

(SeaMonkey :: Themes, defect)

Description

[Hixie (not reading bugmail)]

STEPS TO REPRODUCE
   File | Page Setup (or any other way of getting a dialog with two buttons)
   Focus the Cancel button

ACTUAL RESULT
   Windows Classic: Both buttons are marked as default button (dark outline)
   Modern: OK button is still marked as default

EXPECTED RESULT
   When another button has focus, the default button should lose its default
   button styling since hitting Enter will not activate it.

Comment 1

[Boris Zbarsky [:bzbarsky]]

This seems like a <dialog> issue.... should <dialog> catch focus changes and
twiddle the styling accordingly on buttons?

Comment 2

[Joe Hewitt (gone)]

This behavior shouldn't really be limited to dialog.

Comment 3

[Jonas Jørgensen]

Sec508 subpart B § 1194.21 (c):
"A well-defined on-screen indication of the current focus shall be provided that
moves among interactive interface elements as the input focus changes."

Comment 4

[timeless]

Attachment: comparison (taskbar: run, mozilla: preferences, explorer: folder options)

selected buttons:
taskbar: cancel
mozilla: help
explorer: restore defaults

default buttons is ok in all three dialogs

notice that no button appears to be the default button in folder options, this
is because the dialog is not active.

notice that preferences (also not active) paints two buttons as default (it
should paint zero as default, since it isn't active)

supposing preferences were active, it should behave like the run dialog, only
one button can be default, and if a button is selected, that button is default
and the default default reverts to normal button status.

when i activate the folder options dialog, the only default/selected button
would be restore defaults, ok is just be a normal button.

Comment 5

[Jesse Ruderman]

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

Comment 6

[Asher]

Is anyone working on this at all?  2 years inactive, and it's still a minor
annoyance in the latest Firefox.

Comment 7

[timeless]

[e-mail]: of course not, are you volunteering?

Comment 8

[Asher]

If I could, I would.  I've got plenty of experience with C++, but not with
Mozilla or XUL.  And I wouldn't know where to start.

And I'm not sure if I need to get permission from my employer to work
on the project at all (probably not, as it's IBM).

My hunch is this would be a rather trivial thing for someone to fix if they were
familiar with the code.

Comment 9

[[e-mail]]

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

Comment 10

[OstGote!]

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

Comment 11

[[e-mail]]

Attachment: Proposed patch

Comment 12

[Aaron Leventhal]

Comment on attachment 191480 [details] [diff] [review]
Proposed patch

I'm not a peer but the code looks okay to me.

Comment 13

[Aaron Leventhal]

Comment on attachment 191480 [details] [diff] [review]
Proposed patch

non-peer r= for the accessible-ness of it.

Comment 14

[Asaf Romano (gone)]

Comment on attachment 191480 [details] [diff] [review]
Proposed patch

This has to be #ifdefed out on OS X.

Comment 15

[Mike Connor [:mconnor]]

Comment on attachment 191480 [details] [diff] [review]
Proposed patch

r=me with Mano's comment addressed

Comment 16

[Aaron Leventhal]

Do we have a separate bug for HTML, for darkening the border of the submit
button case? For example, on Google, the default button has a darkened border in
IE and Opera, but not in Firefox -- even when another button isn't focused.

Comment 17

[[e-mail]]

Attachment: Updated for Mac

Comment 18

[Caleb]

(In reply to comment #16)
> Do we have a separate bug for HTML, for darkening the border of the submit
> button case? For example, on Google, the default button has a darkened border in
> IE and Opera, but not in Firefox -- even when another button isn't focused.

I looked and looked but couldn't find it.. the closest I could get to was bug
102057.

I've never paid attention to this problem before to be honest but it does seem
very inconsistent!

Can you file the bug ?

Comment 19

[Asaf Romano (gone)]

Comment on attachment 196640 [details] [diff] [review]
Updated for Mac

>Index: toolkit/content/widgets/dialog.xml
>===================================================================

> #ifdef XP_MACOSX
>       <handler event="keypress" key="." modifiers="meta" phase="capturing" action="this.cancelDialog();"/>
>+#else
>+      <handler event="focus" phase="capturing">
>+        var btn = this.getButton(this.defaultButton);
>+        if (btn)
>+          btn.setAttribute("default", !(event.originalTarget instanceof Components.interfaces.nsIDOMXULButtonElement));
>+      </handler>
>+    </handlers>
> #endif
>     </handlers>

<handlers> is closed outside of the #ifdef. r=mano with that fixed.

Comment 20

[Aaron Leventhal]

(In reply to comment #18)
> (In reply to comment #16)
> > Do we have a separate bug for HTML, for darkening the border of the submit
> > button case? For example, on Google, the default button has a darkened border in
 
> I looked and looked but couldn't find it.. the closest I could get to was bug
> 102057.

I think that is the bug we were looking for.

Comment 21

[[e-mail]]

Attachment: Patch for check in

Comment 22

[timeless]

Comment on attachment 196801 [details] [diff] [review]
Patch for check in

mozilla/toolkit/content/widgets/dialog.xml 	1.26
mozilla/xpfe/global/resources/content/bindings/dialog.xml 	1.35

followup:
mozilla/xpfe/global/resources/content/bindings/dialog.xml 	1.36

Comment 23

[Ian Neal]

Comment on attachment 196801 [details] [diff] [review]
Patch for check in

a=me for SM1.0b on SM only part of code, one more needed

Comment 24

[Christian :Biesinger (don't email me, ping me on IRC)]

a=me for sm1.0b

Comment 25

[Christian :Biesinger (don't email me, ping me on IRC)]

(on the xpfe/ part. and with the followup included)

Comment 26

[[e-mail]]

SeaMonkey-only portion of patch checked in to the 1.8 branch.

Comment 27

[Aaron Leventhal]

Sounds like this is fixed. Shouldn't it be closed now?

Comment 28

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

Comment on attachment 196801 [details] [diff] [review]
Patch for check in

Asking for 1.8.1 approval for the toolkit part.

Comment 29

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

Comment on attachment 196801 [details] [diff] [review]
Patch for check in

mozilla/toolkit/content/widgets/dialog.xml; new revision: 1.25.2.1;