This has been recently added to whatwg html specifications. However, the implementation might be different from what is specified. This may change after beta7.

Are we aiming to land this for 2.0 (and hence need approval)?

(In reply to comment #3) > Are we aiming to land this for 2.0 (and hence need approval)? Part 3 is still WIP (there is a bug when we have <select><option selected>). But very likely, this should be in 2.0 for feature completeness.

I do not really like this patch but it seems to work now. I will try to clean that if I can and write some more tests to be sure I'm not missing something. BTW, if there is <select><option selected>foo</option></select>, it seems that having selected sets make mSelectedIndex change (equals 0 instead of -1) and the validity is checked. However, when this code is called, the value is empty so <select> thinks to suffer from value missing. The simplest fix I've found is to update the validity state when all children are added. I don't really like that :( This code is far from being nice so I will try to found a better way to fix that but I really doubt. Jonas, could you tell me what you think of that patch in the meantime?

Should that be a blocker for completeness of the feature?

Yes, let's finalize this for 4.0 if we can.

Comment on attachment 481810 [details] [diff] [review] Part 3 - <select> can be invalid if no option is selected or all of them have empty value This looks like it's getting the aNotify wrong too :( I really hate the setup we have for this, but I can't think of anything we can do for FF4. Please re-request if you disagree

Inter diff for aNotify changes.

This patch should be aNotify compliant. This thing is *so* annoying. We really need some kind of global variable instead of passing arguments everywhere.

UpdateValueMissingValidityState() needs to take aNotify so that it can forward it to nsHTMLFormElement::UpdateValidity :(

(In reply to comment #12) > UpdateValueMissingValidityState() needs to take aNotify so that it can forward > it to nsHTMLFormElement::UpdateValidity :( See: http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/nsHTMLFormElement.cpp?force=1#1757 According to bz (bug 580575), if we were taking aNotify here, it would be correct to use it for the node it would come from but not for all form elements so it's better to just update for every nodes assuming everything would be fine.

Hmm, a new web facing feature which is a beta N blocker... Not objecting, just pointing this out...

Well, the spec changed under us. @required didn't exist on <select> when Mounir implemented it, but has been added since.

(In reply to comment #15) > Well, the spec changed under us. @required didn't exist on <select> when Mounir > implemented it, but has been added since. Fair enough.

Olli, can you review the change here? I've added a new interface per jst recommandation so I can push that for Gecko 2.0.

Comment on attachment 490851 [details] [diff] [review] Part 1 - Add the required attribute to the select element ># HG changeset patch ># Parent a42e9b001bc86834134c57d15f519a3b67eb8fcf ># User Mounir Lamouri <mounir.lamouri@gmail.com> ># Date 1289911636 -3600 >Bug ??? - r=smaug > >diff --git a/content/html/content/src/nsHTMLSelectElement.cpp b/content/html/content/src/nsHTMLSelectElement.cpp >--- a/content/html/content/src/nsHTMLSelectElement.cpp >+++ b/content/html/content/src/nsHTMLSelectElement.cpp >@@ -177,18 +177,19 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END > NS_IMPL_ADDREF_INHERITED(nsHTMLSelectElement, nsGenericElement) > NS_IMPL_RELEASE_INHERITED(nsHTMLSelectElement, nsGenericElement) > > > DOMCI_NODE_DATA(HTMLSelectElement, nsHTMLSelectElement) > > // QueryInterface implementation for nsHTMLSelectElement > NS_INTERFACE_TABLE_HEAD_CYCLE_COLLECTION_INHERITED(nsHTMLSelectElement) >- NS_HTML_CONTENT_INTERFACE_TABLE3(nsHTMLSelectElement, >+ NS_HTML_CONTENT_INTERFACE_TABLE4(nsHTMLSelectElement, > nsIDOMHTMLSelectElement, >+ nsIDOMHTMLSelectElement_Mozilla20, Not sure whether there is any standard way to mark interfaces to be 2.0 branch. During 1.8 time we marked interface with _1_8_BRANCH So maybe nsIDOMHTMLSelectElement_2_0_BRANCH > nsIDOMHTMLSelectElement.idl \ >+ nsIDOMHTMLSelectElement_Mozilla20.idl \ Too many tabs? >+[scriptable, uuid(9b63d2d3-ccb0-4eed-a9e5-d1c142a805b7)] >+interface nsIDOMHTMLSelectElement_Mozilla20 : nsISupports >+{ >+ attribute boolean required; >+}; >+ Make the interface extend nsIDOMHTMLSelect element. That way you don't need to increase the size of nsHTMLSelectElement objects. (nsHTMLSelectElement should inherit only nsIDOMHTMLSelectElement_2_0_BRANCH) And IMO, there is no need for new file for this temporary trivial interface. Just put it to nsIDOMHTMLSelectElement.idl

Comment on attachment 490865 [details] [diff] [review] Part 1 - Add the required attribute to the select element > // QueryInterface implementation for nsHTMLSelectElement > NS_INTERFACE_TABLE_HEAD_CYCLE_COLLECTION_INHERITED(nsHTMLSelectElement) > NS_HTML_CONTENT_INTERFACE_TABLE3(nsHTMLSelectElement, >- nsIDOMHTMLSelectElement, >+ nsIDOMHTMLSelectElement_Mozilla_2_0_Branch, You can't remove nsIDOMHTMLSelectElement from QI. Just add the new interface to it.

Pushed: http://hg.mozilla.org/mozilla-central/rev/433b919425d6 http://hg.mozilla.org/mozilla-central/rev/ccc6520fee81 http://hg.mozilla.org/mozilla-central/rev/96df19871028

This is already documented: https://developer.mozilla.org/en/HTML/Element/select

If the <select> element is invalid the hint width is equal to field width, which sometimes doesn't look nice, but I guess hints will be replaces with arrow panels, right?

(In reply to comment #23) > If the <select> element is invalid the hint width is equal to field width, > which sometimes doesn't look nice, but I guess hints will be replaces with > arrow panels, right? Yes, we are just waiting for some reviews. It will be done for Firefox 4.