<xul:dialog> should be able to initially disable the accept button

RESOLVED FIXED

Status

()

Core
XUL
--
trivial
RESOLVED FIXED
13 years ago
9 years ago

People

(Reporter: WeirdAl, Assigned: WeirdAl)

Tracking

({fixed-seamonkey1.1a, fixed1.8.1})

Trunk
x86
All
fixed-seamonkey1.1a, fixed1.8.1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

13 years ago
It would be nice to be able to set:

<dialog
  buttons="accept,cancel"
  disableAcceptButton="true">

Then in the dialog.xml binding, we could say:

<xul:button dlgtype="accept" class="dialog-button" 
xbl:inherits="disabled=disableAcceptButton"/>

I'm assigning this bug to myself (it's really easy to fix), but I'd like to see 
if anyone has a better idea to implement this.
(Assignee)

Comment 1

13 years ago
Created attachment 152879 [details] [diff] [review]
<dialog acceptdisabled="true"> patch
(Assignee)

Updated

13 years ago
Attachment #152879 - Flags: review?(timeless)
(Assignee)

Comment 2

13 years ago
Rationale: For most anonymous buttons in a dialog, having them disabled by 
default would not be reasonable.  I believe disabling the accept button would 
be acceptable for semantic reasons.  If a dialog has accept and cancel buttons, 
then the user should be able to cancel without ill effects.  However, the 
accept button is rather more important...

Comment 3

13 years ago
You must be using a really old version of dialog.xml (1.7?); if you look at a
trunk dialog.xml, you'd probably use disabled=buttondisabledaccept instead.
(Assignee)

Comment 4

13 years ago
Hm, that's odd :)  I don't remember the (In reply to comment #3)
> You must be using a really old version of dialog.xml (1.7?); if you look at a
> trunk dialog.xml, you'd probably use disabled=buttondisabledaccept instead.

1.27.  You landed a patch to bug 78274 four days after I created the patch.  :)

That's the fastest case of bit rot I've ever seen.  I'll post a new patch in a
few days.
(Assignee)

Comment 5

13 years ago
Created attachment 171837 [details] [diff] [review]
<dialog buttondisabledaccept="true"> patch

answering neil's concern
Attachment #152879 - Attachment is obsolete: true
Attachment #171837 - Flags: review?(timeless)
(Assignee)

Updated

13 years ago
Attachment #152879 - Flags: review?(timeless)

Updated

13 years ago
Attachment #171837 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #171837 - Flags: review?(timeless)
Attachment #171837 - Flags: review+

Comment 6

13 years ago
please fix the summary :)
(Assignee)

Comment 7

12 years ago
Created attachment 205801 [details] [diff] [review]
patch for toolkit

I'm not entirely sure toolkit wants this one; the xbl:inherits attributes in xpfe don't exist for toolkit.

mconnor, what do you think?
Attachment #205801 - Flags: review?
(Assignee)

Updated

12 years ago
Attachment #205801 - Flags: review? → review?(mconnor)
(Assignee)

Comment 8

12 years ago
Per the SeaMonkey code reviews page, timeless has officially dispensed the sr requirement he originally requested.  In other words, if the SeaMonkey Council has no problem, the xpfe patch can land now.

That said, I am keeping the sr? flag for the moment, pending clarification of the review rules for XUL widget bindings.
(Assignee)

Comment 9

12 years ago
jag, are you willing to offer sr on the xpfe patch?
Comment on attachment 205801 [details] [diff] [review]
patch for toolkit

tbh, I'm not sure why we can't just support this on all buttons, like we do with all other attrs, but I'm struggling to think of a good use case, offhand
Attachment #205801 - Flags: review?(mconnor) → review+
(Assignee)

Comment 11

12 years ago
jag, neil: timeless has r+'d the xpfe version; I'm awaiting sr from one of you (+ or -) on it.  mconnor has given me r+ on the toolkit version.  I'd like to avoid desynchronizing the two versions, so could you please give me a yea or a nay on this?

It's been sitting awaiting sr for over a year now.
(Assignee)

Comment 12

12 years ago
Created attachment 217347 [details] [diff] [review]
patch for xpfe (revised)

Neil points out I missed the mac version.
Attachment #171837 - Attachment is obsolete: true
Attachment #217347 - Flags: superreview?(neil)
Attachment #171837 - Flags: superreview?(neil)

Updated

12 years ago
Attachment #217347 - Flags: superreview?(neil) → superreview+
(Assignee)

Comment 13

12 years ago
checked in by ajschult, thanks everyone :)
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Comment on attachment 205801 [details] [diff] [review]
patch for toolkit

Please land this on the branch as well, a181=mano.
Attachment #205801 - Flags: approval-branch-1.8.1+
Attachment #217347 - Flags: approval-branch-1.8.1?(neil)

Updated

11 years ago
Attachment #217347 - Flags: approval-branch-1.8.1?(neil) → approval-branch-1.8.1+
landed the toolkit patch on the 1.8 branch:
Checking in mozilla/toolkit/content/widgets/dialog.xml;
/cvsroot/mozilla/toolkit/content/widgets/dialog.xml,v  <--  dialog.xml
new revision: 1.25.2.9; previous revision: 1.25.2.8
done
Keywords: fixed1.8.1

Comment 16

11 years ago
Comment on attachment 217347 [details] [diff] [review]
patch for xpfe (revised)

Looks like this was never checked in on the 1.8.1 branch. Neil, can you check it in?

Comment 17

11 years ago
Fix checked in to the branch.
Keywords: fixed-seamonkey1.1a

Updated

9 years ago
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.