33253 2010-01-06 05:51:44 -0800 Want access to <label> element associated with <input> 2023-12-23 09:23:20 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Android Android RESOLVED CONFIGURATION CHANGED P2 Normal --- 0 scroggo webkit-unassigned android-webkit-unforking annevk darin webkit.review.bot yael oldest_to_newest 177573 0 scroggo 2010-01-06 05:51:44 -0800 On Android, we want to use the text in the <label> element as a hint for an <input> element. Although the information is freely available, I found myself copying labelForElement() in AccessibilityRenderObject.cpp. Would it be possible to make this a public function? That way we can call the same function, and if WebKit changes we will not need to make any changes. 177583 1 45962 scroggo 2010-01-06 07:00:58 -0800 Created attachment 45962 Patch making labelForElement a public static function in HTMLLabelElement 177626 2 webkit.review.bot 2010-01-06 09:52:10 -0800 Attachment 45962 did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 Traceback (most recent call last): File "WebKitTools/Scripts/check-webkit-style", line 98, in <module> main() File "WebKitTools/Scripts/check-webkit-style", line 62, in main defaults = style.ArgumentDefaults(style.DEFAULT_OUTPUT_FORMAT, AttributeError: 'module' object has no attribute 'ArgumentDefaults' 177628 3 45962 darin 2010-01-06 09:58:38 -0800 Comment on attachment 45962 Patch making labelForElement a public static function in HTMLLabelElement I'm not entirely sure this is a good change. > +HTMLLabelElement* HTMLLabelElement::labelForElement(Element* element) > +{ > + RefPtr<NodeList> list = element->document()->getElementsByTagName("label"); I know you're just moving the above code, but I'll note that using the NodeList is a quite-inefficient way to iterate all the label elements -- it's the public DOM API for JavaScript use, but inside the engine it's not a great idea. It would be straightforward to just iterate all the nodes in the document looking for label elements instead. It's also really strange that this function uses a node list created by getElementsByTagName but then still checks the tag name of each element in the list. That makes no sense. You just moved the code, but it's not great code. The only real value in having this function in the header would be knowing that the algorithm used is exactly the same one used by HTMLLabelElement itself. And yet I see no evidence that this is true. The function was used only in the accessibility code. It's inefficient to iterate an entire document, so using this function would typically not be encouraged. And it's straightforward to implement this in terms of already existing public functions. > + unsigned len = list->length(); As long as you're moving the code, it would be good to use "length" instead of "len". We prefer words to abbreviations. > Index: WebCore/html/HTMLLabelElement.h > =================================================================== > --- WebCore/html/HTMLLabelElement.h (revision 52806) > +++ WebCore/html/HTMLLabelElement.h (working copy) > @@ -56,6 +56,7 @@ public: > > void focus(bool restorePreviousSelection = true); > > + static HTMLLabelElement* labelForElement(Element* element); > private: > String m_formElementID; There should be a blank line after it so this function isn't paragraphed with the private members below. The argument name "element" should be omitted. I assume you want to make this function public for some kind of Chromium use. Could you be more clear about the specifics? Typically, if there's a function that is not used anywhere in the WebKit source tree people will feel free to refactor so it's private again. So I think it would be best to add the use of the function at the same time you make it public. review- for now 177629 4 darin 2010-01-06 09:59:34 -0800 Ah, not Chromium, Android. It's OK to make this public, but it's also trivial to do this code in WebKit. I'm not entirely sure making this a public function is a good idea. If this was an important operation we'd probably want a more sophisticated approach so we didn't have to iterate the entire document each time. 177699 5 darin 2010-01-06 12:19:59 -0800 (In reply to comment #4) > It's OK to make this public, but it's also trivial to do this code in WebKit. In the WebKit layer rather than in WebCore. 2001747 6 annevk 2023-12-23 09:23:20 -0800 This is now available through IDL. 45962 2010-01-06 07:00:58 -0800 2010-06-11 11:57:34 -0700 Patch making labelForElement a public static function in HTMLLabelElement patch.txt text/plain 4027 scroggo SW5kZXg6IFdlYkNvcmUvYWNjZXNzaWJpbGl0eS9BY2Nlc3NpYmlsaXR5UmVuZGVyT2JqZWN0LmNw cAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL2FjY2Vzc2liaWxpdHkvQWNjZXNzaWJpbGl0eVJlbmRl ck9iamVjdC5jcHAJKHJldmlzaW9uIDUyODA2KQorKysgV2ViQ29yZS9hY2Nlc3NpYmlsaXR5L0Fj Y2Vzc2liaWxpdHlSZW5kZXJPYmplY3QuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC05OTAsMjEgKzk5 MCw2IEBAIFN0cmluZyBBY2Nlc3NpYmlsaXR5UmVuZGVyT2JqZWN0OjphcmlhTGEKICAgICAKICAg ICByZXR1cm4gYWNjZXNzaWJpbGl0eURlc2NyaXB0aW9uRm9yRWxlbWVudHMoZWxlbWVudHMpOwog fQotCi1zdGF0aWMgSFRNTExhYmVsRWxlbWVudCogbGFiZWxGb3JFbGVtZW50KEVsZW1lbnQqIGVs ZW1lbnQpCi17Ci0gICAgUmVmUHRyPE5vZGVMaXN0PiBsaXN0ID0gZWxlbWVudC0+ZG9jdW1lbnQo KS0+Z2V0RWxlbWVudHNCeVRhZ05hbWUoImxhYmVsIik7Ci0gICAgdW5zaWduZWQgbGVuID0gbGlz dC0+bGVuZ3RoKCk7Ci0gICAgZm9yICh1bnNpZ25lZCBpID0gMDsgaSA8IGxlbjsgaSsrKSB7Ci0g ICAgICAgIGlmIChsaXN0LT5pdGVtKGkpLT5oYXNUYWdOYW1lKGxhYmVsVGFnKSkgewotICAgICAg ICAgICAgSFRNTExhYmVsRWxlbWVudCogbGFiZWwgPSBzdGF0aWNfY2FzdDxIVE1MTGFiZWxFbGVt ZW50Kj4obGlzdC0+aXRlbShpKSk7Ci0gICAgICAgICAgICBpZiAobGFiZWwtPmNvcnJlc3BvbmRp bmdDb250cm9sKCkgPT0gZWxlbWVudCkKLSAgICAgICAgICAgICAgICByZXR1cm4gbGFiZWw7Ci0g ICAgICAgIH0KLSAgICB9Ci0gICAgCi0gICAgcmV0dXJuIDA7Ci19CiAgICAgCiBIVE1MTGFiZWxF bGVtZW50KiBBY2Nlc3NpYmlsaXR5UmVuZGVyT2JqZWN0OjpsYWJlbEVsZW1lbnRDb250YWluZXIo KSBjb25zdAogewpAQCAtMTA1MSw3ICsxMDM2LDcgQEAgU3RyaW5nIEFjY2Vzc2liaWxpdHlSZW5k ZXJPYmplY3Q6OnRpdGxlKAogICAgIH0KICAgICAKICAgICBpZiAoaXNJbnB1dFRhZyB8fCBBY2Nl c3NpYmlsaXR5T2JqZWN0Ojppc0FSSUFJbnB1dChhcmlhUm9sZSkgfHwgaXNDb250cm9sKCkpIHsK LSAgICAgICAgSFRNTExhYmVsRWxlbWVudCogbGFiZWwgPSBsYWJlbEZvckVsZW1lbnQoc3RhdGlj X2Nhc3Q8RWxlbWVudCo+KG5vZGUpKTsKKyAgICAgICAgSFRNTExhYmVsRWxlbWVudCogbGFiZWwg PSBIVE1MTGFiZWxFbGVtZW50OjpsYWJlbEZvckVsZW1lbnQoc3RhdGljX2Nhc3Q8RWxlbWVudCo+ KG5vZGUpKTsKICAgICAgICAgaWYgKGxhYmVsICYmICF0aXRsZVVJRWxlbWVudCgpKQogICAgICAg ICAgICAgcmV0dXJuIGxhYmVsLT5pbm5lclRleHQoKTsKICAgICAgICAgCkBAIC0xMTY0LDcgKzEx NDksNyBAQCBJbnRSZWN0IEFjY2Vzc2liaWxpdHlSZW5kZXJPYmplY3Q6OmNoZWNrCiAgICAgaWYg KCFtX3JlbmRlcmVyKQogICAgICAgICByZXR1cm4gSW50UmVjdCgpOwogICAgIAotICAgIEhUTUxM YWJlbEVsZW1lbnQqIGxhYmVsID0gbGFiZWxGb3JFbGVtZW50KHN0YXRpY19jYXN0PEVsZW1lbnQq PihtX3JlbmRlcmVyLT5ub2RlKCkpKTsKKyAgICBIVE1MTGFiZWxFbGVtZW50KiBsYWJlbCA9IEhU TUxMYWJlbEVsZW1lbnQ6OmxhYmVsRm9yRWxlbWVudChzdGF0aWNfY2FzdDxFbGVtZW50Kj4obV9y ZW5kZXJlci0+bm9kZSgpKSk7CiAgICAgaWYgKCFsYWJlbCB8fCAhbGFiZWwtPnJlbmRlcmVyKCkp CiAgICAgICAgIHJldHVybiBib3VuZGluZ0JveFJlY3QoKTsKICAgICAKQEAgLTEzNzcsNyArMTM2 Miw3IEBAIEFjY2Vzc2liaWxpdHlPYmplY3QqIEFjY2Vzc2liaWxpdHlSZW5kZXIKICAgICAgICAg cmV0dXJuIDA7CiAgICAgCiAgICAgTm9kZSogZWxlbWVudCA9IG1fcmVuZGVyZXItPm5vZGUoKTsK LSAgICBIVE1MTGFiZWxFbGVtZW50KiBsYWJlbCA9IGxhYmVsRm9yRWxlbWVudChzdGF0aWNfY2Fz dDxFbGVtZW50Kj4oZWxlbWVudCkpOworICAgIEhUTUxMYWJlbEVsZW1lbnQqIGxhYmVsID0gSFRN TExhYmVsRWxlbWVudDo6bGFiZWxGb3JFbGVtZW50KHN0YXRpY19jYXN0PEVsZW1lbnQqPihlbGVt ZW50KSk7CiAgICAgaWYgKGxhYmVsICYmIGxhYmVsLT5yZW5kZXJlcigpKQogICAgICAgICByZXR1 cm4gYXhPYmplY3RDYWNoZSgpLT5nZXRPckNyZWF0ZShsYWJlbC0+cmVuZGVyZXIoKSk7CiAKQEAg LTI1NTYsNyArMjU0MSw3IEBAIEFjY2Vzc2liaWxpdHlPYmplY3QqIEFjY2Vzc2liaWxpdHlSZW5k ZXIKIAogICAgIE5vZGUqIG5vZGUgPSBtX3JlbmRlcmVyLT5ub2RlKCk7CiAgICAgaWYgKG5vZGUg JiYgbm9kZS0+aXNIVE1MRWxlbWVudCgpKSB7Ci0gICAgICAgIEhUTUxMYWJlbEVsZW1lbnQqIGxh YmVsID0gbGFiZWxGb3JFbGVtZW50KHN0YXRpY19jYXN0PEVsZW1lbnQqPihub2RlKSk7CisgICAg ICAgIEhUTUxMYWJlbEVsZW1lbnQqIGxhYmVsID0gSFRNTExhYmVsRWxlbWVudDo6bGFiZWxGb3JF bGVtZW50KHN0YXRpY19jYXN0PEVsZW1lbnQqPihub2RlKSk7CiAgICAgICAgIGlmIChsYWJlbCkK ICAgICAgICAgICAgIHJldHVybiBheE9iamVjdENhY2hlKCktPmdldE9yQ3JlYXRlKGxhYmVsLT5y ZW5kZXJlcigpKTsKICAgICB9CkluZGV4OiBXZWJDb3JlL2h0bWwvSFRNTExhYmVsRWxlbWVudC5j cHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9odG1sL0hUTUxMYWJlbEVsZW1lbnQuY3BwCShyZXZp c2lvbiA1MjgwNikKKysrIFdlYkNvcmUvaHRtbC9IVE1MTGFiZWxFbGVtZW50LmNwcAkod29ya2lu ZyBjb3B5KQpAQCAtMzAsNiArMzAsNyBAQAogI2luY2x1ZGUgIkV2ZW50TmFtZXMuaCIKICNpbmNs dWRlICJIVE1MRm9ybUVsZW1lbnQuaCIKICNpbmNsdWRlICJIVE1MTmFtZXMuaCIKKyNpbmNsdWRl ICJOb2RlTGlzdC5oIgogCiBuYW1lc3BhY2UgV2ViQ29yZSB7CiAKQEAgLTE2MSw0ICsxNjIsMTkg QEAgdm9pZCBIVE1MTGFiZWxFbGVtZW50OjpzZXRIdG1sRm9yKGNvbnN0IAogICAgIHNldEF0dHJp YnV0ZShmb3JBdHRyLCB2YWx1ZSk7CiB9CiAKK0hUTUxMYWJlbEVsZW1lbnQqIEhUTUxMYWJlbEVs ZW1lbnQ6OmxhYmVsRm9yRWxlbWVudChFbGVtZW50KiBlbGVtZW50KQoreworICAgIFJlZlB0cjxO b2RlTGlzdD4gbGlzdCA9IGVsZW1lbnQtPmRvY3VtZW50KCktPmdldEVsZW1lbnRzQnlUYWdOYW1l KCJsYWJlbCIpOworICAgIHVuc2lnbmVkIGxlbiA9IGxpc3QtPmxlbmd0aCgpOworICAgIGZvciAo dW5zaWduZWQgaSA9IDA7IGkgPCBsZW47IGkrKykgeworICAgICAgICBpZiAobGlzdC0+aXRlbShp KS0+aGFzVGFnTmFtZShsYWJlbFRhZykpIHsKKyAgICAgICAgICAgIEhUTUxMYWJlbEVsZW1lbnQq IGxhYmVsID0gc3RhdGljX2Nhc3Q8SFRNTExhYmVsRWxlbWVudCo+KGxpc3QtPml0ZW0oaSkpOwor ICAgICAgICAgICAgaWYgKGxhYmVsLT5jb3JyZXNwb25kaW5nQ29udHJvbCgpID09IGVsZW1lbnQp CisgICAgICAgICAgICAgICAgcmV0dXJuIGxhYmVsOworICAgICAgICB9CisgICAgfQorCisgICAg cmV0dXJuIDA7Cit9CisKIH0gLy8gbmFtZXNwYWNlCkluZGV4OiBXZWJDb3JlL2h0bWwvSFRNTExh YmVsRWxlbWVudC5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvaHRtbC9IVE1MTGFiZWxFbGVtZW50 LmgJKHJldmlzaW9uIDUyODA2KQorKysgV2ViQ29yZS9odG1sL0hUTUxMYWJlbEVsZW1lbnQuaAko d29ya2luZyBjb3B5KQpAQCAtNTYsNiArNTYsNyBAQCBwdWJsaWM6CiAKICAgICB2b2lkIGZvY3Vz KGJvb2wgcmVzdG9yZVByZXZpb3VzU2VsZWN0aW9uID0gdHJ1ZSk7CiAKKyAgICBzdGF0aWMgSFRN TExhYmVsRWxlbWVudCogbGFiZWxGb3JFbGVtZW50KEVsZW1lbnQqIGVsZW1lbnQpOwogIHByaXZh dGU6CiAgICAgU3RyaW5nIG1fZm9ybUVsZW1lbnRJRDsKIH07Cg==