Align td.rowSpan / td.colSpan with the specification: https://html.spec.whatwg.org/multipage/tables.html#htmltablecellelement
Created attachment 272469 [details] Patch
Comment on attachment 272469 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=272469&action=review Really would be nice to make fastGetAttribute and setAttributeWithoutSynchronization have names that are more similar. They both seem to be dealing with the same issue, but they use entirely different terminology. > Source/WebCore/html/HTMLTableCellElement.cpp:45 > +unsigned HTMLTableCellElement::colSpan() const I think we should rename this at some point. It’s getting pretty far from “colSpan”, it’s more like “effectiveColSpan” or something like that. Or we could name it something explicit without abbreviation. I wonder how many different call sites there even are? Maybe eventually colSpanForBindings can be renamed back and this additional rule (1 or more) can be put into the table logic, not the HTMLTableCellElement class. > Source/WebCore/html/HTMLTableCellElement.cpp:55 > +unsigned HTMLTableCellElement::rowSpan() const I think we should rename this at some point. It’s getting pretty far from “rolSpan”, it’s more like “effectiveRolSpan” or something like that. Or we could name it something explicit without abbreviation. I wonder how many different call sites there even are? Maybe eventually rolSpanForBindings can be renamed back and the additional rules (1 or more, apparently unwanted, maximum of 8190) can be put into the table logic, not the HTMLTableCellElement class. > Source/WebCore/html/HTMLTableCellElement.cpp:57 > + static const unsigned maxRowspan = 8190; Would be nice to have a comment explaining the choice of 8190. “To match Firefox” was not a very good explanation. But what would the explanation be? > Source/WebCore/html/HTMLTableCellElement.h:43 > - void setColSpan(int); > - void setRowSpan(int); > + void setColSpanForBindings(unsigned); > + void setRowSpanForBindings(unsigned); Unfortunate renaming here forced by the renaming of the getters, I guess.
Comment on attachment 272469 [details] Patch Clearing flags on attachment: 272469 Committed r197354: <http://trac.webkit.org/changeset/197354>
All reviewed patches have been landed. Closing bug.