| Summary: | When we do setAttribute("border", null) on a table we should create a border like every other browser | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Robert Hogan <robert> | ||||||||||||
| Component: | Layout and Rendering | Assignee: | Robert Hogan <robert> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | buildbot, glenn, ian, jchaffraix, Ms2ger, ojan.autocc, rniwa, robert, webkit.review.bot, xn--mlform-iua | ||||||||||||
| Priority: | P2 | ||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| Attachments: |
|
||||||||||||||
Created attachment 173938 [details]
Reduction
this is correct behavior per DOM4 [1] and WebIDL [2] specs; [1] http://www.w3.org/TR/domcore/#element [2] http://www.w3.org/TR/WebIDL/#TreatNullAs if setAttribute() were redefined as: void setAttribute(DOMString name, [TreatNullAs=EmptyString] DOMString value); then the behavior you observe would be a bug, but at present it is not; it is also not related to CSS you need to take this up with the W3C (and make sure other browser behave as you suggest) before reopening this; in the mean time, i'm marking as WORKSFORME (In reply to comment #2) > this is correct behavior per DOM4 [1] and WebIDL [2] specs; > > [1] http://www.w3.org/TR/domcore/#element > [2] http://www.w3.org/TR/WebIDL/#TreatNullAs > > if setAttribute() were redefined as: > So per the spec we're correct to stringify null when setting an attribute. But we're the only browser that regards setAttribute("border", null) as not setting a border so we should fix that here I guess. Created attachment 180736 [details]
Patch
border="null" is equivalent to border="1" according to the HTML standard; see near the end of: http://whatwg.org/html/#tables Note that the current (most up to date) URL for the DOM4 spec is: http://dom.spec.whatwg.org/ Created attachment 183234 [details]
Patch
Comment on attachment 183234 [details] Patch Attachment 183234 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://queues.webkit.org/results/15942137 Comment on attachment 183234 [details] Patch Attachment 183234 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15914982 Created attachment 183276 [details]
Patch
Comment on attachment 183276 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183276&action=review > Source/WebCore/html/HTMLElement.cpp:140 > + bool isOK = false; I don't think isOK is a descriptive name. How about parsedCorrectly? > Source/WebCore/html/HTMLElement.cpp:143 > + return (!isOK && hasLocalName(tableTag)) ? 1 : borderWidth; No need for parenthesis around the condition. Personally, I would have done: if (value.isEmpty() || !parseHTMLNonNegativeInteger(value, borderWidth)) return 1; return borderWidth. instead. > LayoutTests/fast/dom/HTMLTableElement/table-with-invalid-border.html:1 > +<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd"> Why are we using HTML4.01 DOCTYPE as opposed to HTML5 DOCTYPE? > LayoutTests/fast/dom/HTMLTableElement/table-with-invalid-border.html:14 > + <table id="table" border="djska" cellpadding="5" cellspacing="0" width="100%" height="100%"> > + <tr> > + <td id="cell3" height="26"></td> Why do we need to have ids and cellpadding, etc…? > LayoutTests/fast/dom/HTMLTableElement/table-with-invalid-border.html:20 > + shouldBeEqualToString('document.defaultView.getComputedStyle(table, null).getPropertyValue("border-top-width")', "1px"); Please explicitly get the table via document.querySelector('table') for example. Named properties on window object is a WebKit extension. Created attachment 183817 [details]
Patch
Comment on attachment 183817 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183817&action=review > Source/WebCore/html/HTMLTableElement.cpp:377 > // FIXME: This attribute is a mess. Maybe remove this comment? Comment on attachment 183817 [details] Patch Clearing flags on attachment: 183817 Committed r140436: <http://trac.webkit.org/changeset/140436> All reviewed patches have been landed. Closing bug. *** Bug 56982 has been marked as a duplicate of this bug. *** |
When we do setAttribute("border", null), the Attribute object interprets null as the string "null"