|Summary:||HTMLLinkElement.disabled does not forward value to the Stylesheet's disabled attribute on setting|
|Product:||WebKit||Reporter:||Bijan Amirzada <bijana>|
|Component:||CSS||Assignee:||Julien Chaffraix <jchaffraix>|
|Severity:||Trivial||CC:||ap, bijana, commit-queue, jamesr, jchaffraix, seasoup, tnainani, webkit.review.bot|
|Version:||528+ (Nightly build)|
|Bug Depends on:|
Description Bijan Amirzada 2010-09-08 17:32:34 PDT
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.
Comment 2 WebKit Review Bot 2010-09-15 10:41:53 PDT
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]  Total errors found: 1 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Bijan Amirzada 2010-09-15 14:58:51 PDT
Created attachment 67721 [details] Updated Patch - per style bot mod request
Comment 4 Bijan Amirzada 2010-09-15 14:59:33 PDT
Comment on attachment 67210 [details] Updated Patch This patch is now obsolete.
Comment 5 Darin Adler 2010-10-13 17:15:43 PDT
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.
Comment 6 Alexey Proskuryakov 2011-02-17 11:13:38 PST
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.
Comment 7 Julien Chaffraix 2011-04-08 17:20:17 PDT
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 8 WebKit Commit Bot 2011-04-19 19:13:51 PDT
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>
Comment 9 WebKit Commit Bot 2011-04-19 19:13:55 PDT
All reviewed patches have been landed. Closing bug.
Comment 10 James Robinson 2011-05-25 16:43:48 PDT
This appears to have caused https://bugs.webkit.org/show_bug.cgi?id=61400
Comment 11 Alexey Proskuryakov 2011-11-10 09:05:40 PST
This bug is still RESOLVED/FIXED, but it looks like the changes have been reverted in r94369. Should it be reopened?
Comment 12 Alexey Proskuryakov 2011-11-10 09:07:34 PST
If we want to track this as bug 67436 now, maybe this bug should be changed to INVALID?