Last Comment Bug 247849 - <xul:dialog> should be able to initially disable the accept button
: <xul:dialog> should be able to initially disable the accept button
Status: RESOLVED FIXED
: fixed-seamonkey1.1a, fixed1.8.1
Product: Core
Classification: Components
Component: XUL (show other bugs)
: Trunk
: x86 All
: -- trivial (vote)
: ---
Assigned To: Alex Vincent [:WeirdAl]
:
: Neil Deakin
Mentors:
http://lxr.mozilla.org/seamonkey/sour...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-06-20 16:01 PDT by Alex Vincent [:WeirdAl]
Modified: 2008-07-31 03:15 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
<dialog acceptdisabled="true"> patch (789 bytes, patch)
2004-07-11 15:02 PDT, Alex Vincent [:WeirdAl]
no flags Details | Diff | Splinter Review
<dialog buttondisabledaccept="true"> patch (1.44 KB, patch)
2005-01-19 19:21 PST, Alex Vincent [:WeirdAl]
timeless: review+
Details | Diff | Splinter Review
patch for toolkit (1.70 KB, patch)
2005-12-13 23:16 PST, Alex Vincent [:WeirdAl]
mconnor: review+
asaf: approval‑branch‑1.8.1+
Details | Diff | Splinter Review
patch for xpfe (revised) (3.68 KB, patch)
2006-04-05 15:57 PDT, Alex Vincent [:WeirdAl]
neil: superreview+
neil: approval‑branch‑1.8.1+
Details | Diff | Splinter Review

Description Alex Vincent [:WeirdAl] 2004-06-20 16:01:57 PDT
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.
Comment 1 Alex Vincent [:WeirdAl] 2004-07-11 15:02:40 PDT
Created attachment 152879 [details] [diff] [review]
<dialog acceptdisabled="true"> patch
Comment 2 Alex Vincent [:WeirdAl] 2004-07-11 15:08:27 PDT
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 neil@parkwaycc.co.uk 2004-12-30 13:35:07 PST
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.
Comment 4 Alex Vincent [:WeirdAl] 2004-12-30 13:45:00 PST
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.
Comment 5 Alex Vincent [:WeirdAl] 2005-01-19 19:21:27 PST
Created attachment 171837 [details] [diff] [review]
<dialog buttondisabledaccept="true"> patch

answering neil's concern
Comment 6 timeless 2005-02-06 13:38:58 PST
please fix the summary :)
Comment 7 Alex Vincent [:WeirdAl] 2005-12-13 23:16:26 PST
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?
Comment 8 Alex Vincent [:WeirdAl] 2005-12-13 23:40:32 PST
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.
Comment 9 Alex Vincent [:WeirdAl] 2005-12-14 16:26:57 PST
jag, are you willing to offer sr on the xpfe patch?
Comment 10 Mike Connor [:mconnor] 2006-04-05 13:35:13 PDT
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
Comment 11 Alex Vincent [:WeirdAl] 2006-04-05 13:57:04 PDT
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.
Comment 12 Alex Vincent [:WeirdAl] 2006-04-05 15:57:14 PDT
Created attachment 217347 [details] [diff] [review]
patch for xpfe (revised)

Neil points out I missed the mac version.
Comment 13 Alex Vincent [:WeirdAl] 2006-04-05 19:29:20 PDT
checked in by ajschult, thanks everyone :)
Comment 14 Mano (::mano, needinfo? for any questions; not reading general bugmail) 2006-05-31 02:10:31 PDT
Comment on attachment 205801 [details] [diff] [review]
patch for toolkit

Please land this on the branch as well, a181=mano.
Comment 15 Mano (::mano, needinfo? for any questions; not reading general bugmail) 2006-06-10 07:08:12 PDT
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
Comment 16 Stefan [:stefanh] (away until December 6) 2006-06-11 10:52:52 PDT
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 neil@parkwaycc.co.uk 2006-06-11 12:27:46 PDT
Fix checked in to the branch.

Note You need to log in before you can comment on or make changes to this bug.