Closed Bug 247849 Opened 20 years ago Closed 18 years ago

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

Categories

(Core :: XUL, defect)

x86
All
defect
Not set
trivial

Tracking

()

RESOLVED FIXED

People

(Reporter: WeirdAl, Assigned: WeirdAl)

References

()

Details

(Keywords: fixed-seamonkey1.1a, fixed1.8.1)

Attachments

(2 files, 2 obsolete files)

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.
Attachment #152879 - Flags: review?(timeless)
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...
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.
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.
answering neil's concern
Attachment #152879 - Attachment is obsolete: true
Attachment #171837 - Flags: review?(timeless)
Attachment #152879 - Flags: review?(timeless)
Attachment #171837 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #171837 - Flags: review?(timeless)
Attachment #171837 - Flags: review+
please fix the summary :)
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?
Attachment #205801 - Flags: review? → review?(mconnor)
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.
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+
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.
Neil points out I missed the mac version.
Attachment #171837 - Attachment is obsolete: true
Attachment #217347 - Flags: superreview?(neil)
Attachment #171837 - Flags: superreview?(neil)
Attachment #217347 - Flags: superreview?(neil) → superreview+
checked in by ajschult, thanks everyone :)
Status: NEW → RESOLVED
Closed: 18 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)
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 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?
Fix checked in to the branch.
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.