Created attachment 66970 [details] Patch +++ This bug was initially created as a clone of Bug #25287 +++ See testcase: attached Per the html5 spec 4.2.7 - The disabled IDL attribute on link and style elements must return false and do nothing on setting, if the sheet attribute of their LinkStyle interface is null. Otherwise, it must return the value of the StyleSheet interface's disabled attribute on getting, and forward the new value to that same attribute on setting. Currently, the HTMLLinkElement.disabled is not forwarding the new value to the Stylesheet's disabled attribute on setting.
Created attachment 67210 [details] Updated Patch
Attachment 67210 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/html/HTMLLinkElement.cpp:393: An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] Total errors found: 1 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 67721 [details] Updated Patch - per style bot mod request
Comment on attachment 67210 [details] Updated Patch This patch is now obsolete.
Comment on attachment 67721 [details] Updated Patch - per style bot mod request View in context: https://bugs.webkit.org/attachment.cgi?id=67721&action=review Looks good. Nice test case. Some small stylistic problems that we should fix. > LayoutTests/fast/html/htmllink-disable-expected.txt:1 > +This test checks HTMLLinkElement disabled conforms to HTML5 spec for the bug id #45425 . A slightly better directory for this test would be: fast/dom/HTMLLinkElement You’ll find other tests of HTMLLinkElement DOM API there. > WebCore/html/HTMLLinkElement.cpp:397 > + if (StyleSheet* styleSheet = this->sheet()) > + return styleSheet->disabled(); > + > + return false; In WebKit we normally use the early return style rather than nesting code. Also, the "this->" notation would only be needed if the local variable was named the same as the data member. There’s also no reason to use a non-inline function to get one of our own data members. This function should just say: return m_sheet && m_sheet->disabled(); > WebCore/html/HTMLLinkElement.cpp:408 > + if (StyleSheet* styleSheet = this->sheet()) > + styleSheet->setDisabled(setDisabled); > +} > + > + > + > } It doesn’t make sense to name the argument “set disabled”. You could name it “disabled” or “flag” or something like that. This function should just be: if (!m_sheet) return; m_sheet->setDisabled(disabled); Just one blank line after the function, please, not three. > WebCore/html/HTMLLinkElement.h:76 > + bool disabled(); This could and should be a const member function.
Created attachment 82832 [details] with style fixes I tried to update this patch for landing, but it actually breaks a regression test, fast/dom/boolean-attribute-reflection.html.
Created attachment 88904 [details] Upated the patch on top of ToT - removed testing for boolean reflected attribute on link:disabled per HTML5 standard & other browsers' behavior
Comment on attachment 88904 [details] Upated the patch on top of ToT - removed testing for boolean reflected attribute on link:disabled per HTML5 standard & other browsers' behavior Clearing flags on attachment: 88904 Committed r84329: <http://trac.webkit.org/changeset/84329>
All reviewed patches have been landed. Closing bug.
This appears to have caused https://bugs.webkit.org/show_bug.cgi?id=61400
This bug is still RESOLVED/FIXED, but it looks like the changes have been reverted in r94369. Should it be reopened?
If we want to track this as bug 67436 now, maybe this bug should be changed to INVALID?
(In reply to comment #12) > If we want to track this as bug 67436 now, maybe this bug should be changed to INVALID? That's the idea. Sorry for putting the wrong state.