Align HTMLAreaElement shape parsing with the specification: - https://html.spec.whatwg.org/#attr-area-shape This also aligns our behavior with Chrome.
Created attachment 288156 [details] Patch
Comment on attachment 288156 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=288156&action=review > Source/WebCore/ChangeLog:12 > + In particular, add support for circ / polygon non conforming shapes > + and use rectangle as default state. How did you come to the decision to add support for such "non conforming" shapes? I know that adding such support makes us pass more tests and matches Chrome behavior. Should non-conforming shapes be recognized in standards mode (as opposed to quirks mode)? Does this agree with other browsers, including Firefox and the latest IE/Edge? Are we expected to add support for such non-conforming shapes? I am unclear of our obligation to implement such non-conforming shapes. The wording in "Errors involving limits that have been imposed merely to simplify the language" of section "Restrictions on content models and on attribute values" of the HTML living standard, <https://html.spec.whatwg.org/#restrictions-on-content-models-and-on-attribute-values>, seems to imply that we should support "circ" because it is easier to teach? As far as I can tell circ, polygon, rectangle are Microsoft extensions that are documented at <https://msdn.microsoft.com/en-us/library/ms534628(v=vs.85).aspx>. > Source/WebCore/html/HTMLAreaElement.cpp:64 > - else if (equalLettersIgnoringASCIICase(value, "rect")) > + else { > + // The missing value default is the rectangle state. > m_shape = Rect; > + } Please add tests for shape "rectangle" and the "missing value" case.
Comment on attachment 288156 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=288156&action=review >> Source/WebCore/html/HTMLAreaElement.cpp:64 >> + } > > Please add tests for shape "rectangle" and the "missing value" case. Disregard the remark about adding a test for the "missing value" case. Please also add a test for shape "polygon".
(In reply to comment #2) > Comment on attachment 288156 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=288156&action=review > > > Source/WebCore/ChangeLog:12 > > + In particular, add support for circ / polygon non conforming shapes > > + and use rectangle as default state. > > How did you come to the decision to add support for such "non conforming" > shapes? I know that adding such support makes us pass more tests and matches > Chrome behavior. Should non-conforming shapes be recognized in standards > mode (as opposed to quirks mode)? Does this agree with other browsers, > including Firefox and the latest IE/Edge? Are we expected to add support for > such non-conforming shapes? I am unclear of our obligation to implement such > non-conforming shapes. The wording in "Errors involving limits that have > been imposed merely to simplify the language" of section "Restrictions on > content models and on attribute values" of the HTML living standard, > <https://html.spec.whatwg.org/#restrictions-on-content-models-and-on- > attribute-values>, seems to imply that we should support "circ" because it > is easier to teach? Because: 1. It is in the HTML spec 2. It is covered by W3C tests 3. It is supported by Chrome. > > As far as I can tell circ, polygon, rectangle are Microsoft extensions that > are documented at > <https://msdn.microsoft.com/en-us/library/ms534628(v=vs.85).aspx>. > > > Source/WebCore/html/HTMLAreaElement.cpp:64 > > - else if (equalLettersIgnoringASCIICase(value, "rect")) > > + else { > > + // The missing value default is the rectangle state. > > m_shape = Rect; > > + } > > Please add tests for shape "rectangle" and the "missing value" case. Ok, I'll extend testing.
(In reply to comment #4) > (In reply to comment #2) > > Comment on attachment 288156 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=288156&action=review > > > > > Source/WebCore/ChangeLog:12 > > > + In particular, add support for circ / polygon non conforming shapes > > > + and use rectangle as default state. > > > > How did you come to the decision to add support for such "non conforming" > > shapes? I know that adding such support makes us pass more tests and matches > > Chrome behavior. Should non-conforming shapes be recognized in standards > > mode (as opposed to quirks mode)? Does this agree with other browsers, > > including Firefox and the latest IE/Edge? Are we expected to add support for > > such non-conforming shapes? I am unclear of our obligation to implement such > > non-conforming shapes. The wording in "Errors involving limits that have > > been imposed merely to simplify the language" of section "Restrictions on > > content models and on attribute values" of the HTML living standard, > > <https://html.spec.whatwg.org/#restrictions-on-content-models-and-on- > > attribute-values>, seems to imply that we should support "circ" because it > > is easier to teach? > > Because: > 1. It is in the HTML spec > 2. It is covered by W3C tests > 3. It is supported by Chrome. > > > > > As far as I can tell circ, polygon, rectangle are Microsoft extensions that > > are documented at > > <https://msdn.microsoft.com/en-us/library/ms534628(v=vs.85).aspx>. > > > > > Source/WebCore/html/HTMLAreaElement.cpp:64 > > > - else if (equalLettersIgnoringASCIICase(value, "rect")) > > > + else { > > > + // The missing value default is the rectangle state. > > > m_shape = Rect; > > > + } > > > > Please add tests for shape "rectangle" and the "missing value" case. > > Ok, I'll extend testing. Actually, the "missing value" case is also covered in the test I rebaselined. "rectangle" is not tested but even if not supposed it would fallback to using Rect shape by default.
(In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #2) > > > Comment on attachment 288156 [details] > > > Patch > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=288156&action=review > > > > > > > Source/WebCore/ChangeLog:12 > > > > + In particular, add support for circ / polygon non conforming shapes > > > > + and use rectangle as default state. > > > > > > How did you come to the decision to add support for such "non conforming" > > > shapes? I know that adding such support makes us pass more tests and matches > > > Chrome behavior. Should non-conforming shapes be recognized in standards > > > mode (as opposed to quirks mode)? Does this agree with other browsers, > > > including Firefox and the latest IE/Edge? Are we expected to add support for > > > such non-conforming shapes? I am unclear of our obligation to implement such > > > non-conforming shapes. The wording in "Errors involving limits that have > > > been imposed merely to simplify the language" of section "Restrictions on > > > content models and on attribute values" of the HTML living standard, > > > <https://html.spec.whatwg.org/#restrictions-on-content-models-and-on- > > > attribute-values>, seems to imply that we should support "circ" because it > > > is easier to teach? > > > > Because: > > 1. It is in the HTML spec > > 2. It is covered by W3C tests > > 3. It is supported by Chrome. > > > > > > > > As far as I can tell circ, polygon, rectangle are Microsoft extensions that > > > are documented at > > > <https://msdn.microsoft.com/en-us/library/ms534628(v=vs.85).aspx>. > > > > > > > Source/WebCore/html/HTMLAreaElement.cpp:64 > > > > - else if (equalLettersIgnoringASCIICase(value, "rect")) > > > > + else { > > > > + // The missing value default is the rectangle state. > > > > m_shape = Rect; > > > > + } > > > > > > Please add tests for shape "rectangle" and the "missing value" case. > > > > Ok, I'll extend testing. > > Actually, the "missing value" case is also covered in the test I > rebaselined. "rectangle" is not tested but even if not supposed it would > fallback to using Rect shape by default. if not *supported*. As a matter of fact, my code change does not explicitly test for "rectangle".
Created attachment 288170 [details] Patch
Comment on attachment 288170 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=288170&action=review > Source/WebCore/ChangeLog:14 > + This also aligns our behavior with Chrome. A more compelling argument is that "circ", "polygon" and "rectangle" have appeared in various publications, including in O'Reilly's HTML & XHTML: The Definitive Guide.
Comment on attachment 288170 [details] Patch Clearing flags on attachment: 288170 Committed r205562: <http://trac.webkit.org/changeset/205562>
All reviewed patches have been landed. Closing bug.