Summary: | Align Document.body setter with the HTML specification | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||
Component: | DOM | Assignee: | Chris Dumez <cdumez> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | cdumez, commit-queue, darin, esprehn+autocc, kangil.han, rniwa | ||||||||
Priority: | P2 | Keywords: | WebExposed | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
URL: | https://html.spec.whatwg.org/multipage/dom.html#dom-document-body | ||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=159488 | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 119115 | ||||||||||
Attachments: |
|
Description
Chris Dumez
2016-07-06 14:08:03 PDT
Created attachment 282954 [details]
Patch
Created attachment 282955 [details]
Patch
Comment on attachment 282955 [details]
Patch
It seems like you should make newBody a Ref or keep the null check.
(In reply to comment #3) > Comment on attachment 282955 [details] > Patch > > It seems like you should make newBody a Ref or keep the null check. Why? if (!is<HTMLBodyElement>(newBody.get()) && !is<HTMLFrameSetElement>(newBody.get())) Will be true if newBody is null and therefore we will throw a HIERARCHY_REQUEST_ERR as expected. We cannot easily take a Ref<> because this function is used by the bindings and the type is nullable in the IDL. If we updated the type in the bindings to be non-nullable then we would no longer match the spec and we would throw a TypeError instead of a HIERARCHY_REQUEST_ERR. Created attachment 282981 [details]
Patch
(In reply to comment #4) > (In reply to comment #3) > > Comment on attachment 282955 [details] > > Patch > > > > It seems like you should make newBody a Ref or keep the null check. > > Why? > > if (!is<HTMLBodyElement>(newBody.get()) && > !is<HTMLFrameSetElement>(newBody.get())) > Will be true if newBody is null and therefore we will throw a > HIERARCHY_REQUEST_ERR as expected. > > We cannot easily take a Ref<> because this function is used by the bindings > and the type is nullable in the IDL. If we updated the type in the bindings > to be non-nullable then we would no longer match the spec and we would throw > a TypeError instead of a HIERARCHY_REQUEST_ERR. I added a test case to cover "document.body = null" to confirm that it still throws a HIERARCHY_REQUEST_ERR. Comment on attachment 282981 [details]
Patch
Sure enough. I should be more familiar with is<>. r=me
That spec is very specific, and now our code reads like the spec.
Comment on attachment 282981 [details] Patch Clearing flags on attachment: 282981 Committed r202893: <http://trac.webkit.org/changeset/202893> All reviewed patches have been landed. Closing bug. |