Closed Bug 1172086 Opened 5 years ago Closed 5 years ago
Empty, but disabled <input> with associated <datalist> incorrectly allows input
See the example testcase  Initially tapping the disabled <input> provides the dropdown datalist, and allows item selection via tap. Interestingly, once the field is populated, subsequent tapping behaviour is as expected (not allowed).  https://www.dropbox.com/s/5xl2sm7ehcpdu95/bug_Disabled_Input_wDatalist.html?dl=0
Can I take it?
The bug will be assigned to whoever can attach a viable patch for review. If you've completed the requirements as in comment #1, please post your work.
Here there is a proposed patch. The attribute disabled (http://www.w3.org/TR/html5/forms.html#attr-fe-disabled) is a boolean attribute (http://www.w3.org/TR/html5/infrastructure.html#boolean-attribute). I quote "The presence of a boolean attribute on an element represents the true value, and the absence of the attribute represents the false value." Then it is sufficient to check if there is or not the attribute using the method .hasAttribute . Is it right? I built and check the patch on my android device and seems to work.
Close! We normally prefer the use of the element property vs. it's attribute ... can you try |aElement.disabled| ? This is normally preferred as simpler and easier to quickly read, and also sometimes for visible controls actually more correct. See additional reading , and .  https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/Attribute/disabled  https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes#Content_versus_IDL_attributes Also, a slight style "nit" for your patch, I don't think it'll be necessary to wrap the new condition in parens |(aElement.hasAttribute("disabled"))| vs. |aElement.hasAttribute("disabled")| (See the earlier free-standing condition for |aElement.readOnly|)
Assignee: nobody → p.calligaris.code
Status: NEW → ASSIGNED
I updated the patch as you suggested. Thank you for the useful links!
Attachment #8616384 - Attachment is obsolete: true
This looks fine, and wfm locally ... let's push to TRY / automated test-farm. https://treeherder.mozilla.org/#/jobs?repo=try&revision=f6a286029b55 If this works ok, we'll have you change |r=reviewers| to |r=margaret| in your patch commit message, and re-post flagging her for review=?
Ok, (as expected), this looks fine. As mentioned in the previous comment, go ahead and: 1) change |r=reviewers| to |r=margaret| in your patch commit message 2) repost the new patch, and flag it for her review
Last proposed patch. Thanks for the help Mark!
Comment on attachment 8616875 [details] [diff] [review] bug1172086_input_element_disabled_property_issue.patch Review of attachment 8616875 [details] [diff] [review]: ----------------------------------------------------------------- Looks great, thanks! And thanks to capella for mentoring!
Attachment #8616875 - Flags: review?(margaret.leibovic) → review+
Whiteboard: [lang=js][good first bug] → [lang=js][good first bug][fixed-in-fx-team]
You need to log in before you can comment on or make changes to this bug.