Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(154)

Issue 366883003: Make DOMImplementation.createHTMLDocument implementation more spec aligned (Closed)

Created:
6 years, 5 months ago by kangil_
Modified:
6 years, 5 months ago
Reviewers:
haraken, tkent
CC:
blink-reviews, blink-reviews-dom_chromium.org, dglazkov+blink, sof, eae+blinkwatch, rwlbuis
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Make DOMImplementation.createHTMLDocument implementation more spec aligned According to specification, if the title argument is not omitted: *)Create a title element in the HTML namespace, and append it to the head element created in the previous step. *)Create a Text node, set its data to title (which could be the empty string), and append it to the title element created in the previous step. spec: http://dom.spec.whatwg.org/#dom-domimplementation-createhtmldocument So this CL modified implementation from setTitle to create and append. TEST=LayoutTests/fast/dom/implementation-createHTMLDocument.html Additionally, with this CL, DOMImplementation.createHTMLDocument test at http://w3c-test.org/dom/nodes/DOMImplementation-createHTMLDocument.html will be passed. Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=177379

Patch Set 1 #

Total comments: 4

Patch Set 2 : Take review comment into consideration #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+13 lines, -3 lines) Patch
M LayoutTests/fast/dom/implementation-createHTMLDocument.html View 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/fast/dom/implementation-createHTMLDocument-expected.txt View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/dom/DOMImplementation.cpp View 1 2 chunks +11 lines, -3 lines 2 comments Download

Messages

Total messages: 13 (0 generated)
kangil_
PTAL, thx!
6 years, 5 months ago (2014-07-02 06:57:03 UTC) #1
haraken
Does this behavior align with Firefox and IE?
6 years, 5 months ago (2014-07-02 07:51:06 UTC) #2
kangil_
On 2014/07/02 07:51:06, haraken wrote: > Does this behavior align with Firefox and IE? FF: ...
6 years, 5 months ago (2014-07-02 07:54:03 UTC) #3
haraken
On 2014/07/02 07:54:03, kangil_ wrote: > On 2014/07/02 07:51:06, haraken wrote: > > Does this ...
6 years, 5 months ago (2014-07-02 08:02:29 UTC) #4
kangil_
PTAL, thx! https://codereview.chromium.org/366883003/diff/1/Source/core/dom/DOMImplementation.cpp File Source/core/dom/DOMImplementation.cpp (right): https://codereview.chromium.org/366883003/diff/1/Source/core/dom/DOMImplementation.cpp#newcode331 Source/core/dom/DOMImplementation.cpp:331: if (!title.isNull()) { On 2014/07/02 08:02:29, haraken ...
6 years, 5 months ago (2014-07-02 08:23:12 UTC) #5
haraken
+tkent LGTM
6 years, 5 months ago (2014-07-02 08:26:40 UTC) #6
kangil_
The CQ bit was checked by kangil.han@samsung.com
6 years, 5 months ago (2014-07-02 09:07:51 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kangil.han@samsung.com/366883003/20001
6 years, 5 months ago (2014-07-02 09:08:55 UTC) #8
commit-bot: I haz the power
Change committed as 177379
6 years, 5 months ago (2014-07-02 13:21:51 UTC) #9
tkent
What about other browser behavior? I feel the createHTMLDocument in the DOM specification and Document.title ...
6 years, 5 months ago (2014-07-04 01:18:28 UTC) #10
haraken
On 2014/07/04 01:18:28, tkent wrote: > What about other browser behavior? > I feel the ...
6 years, 5 months ago (2014-07-04 01:26:42 UTC) #11
kangil_
https://codereview.chromium.org/366883003/diff/20001/Source/core/dom/DOMImplementation.cpp File Source/core/dom/DOMImplementation.cpp (right): https://codereview.chromium.org/366883003/diff/20001/Source/core/dom/DOMImplementation.cpp#newcode336 Source/core/dom/DOMImplementation.cpp:336: titleElement->appendChild(d->createTextNode(title), IGNORE_EXCEPTION); On 2014/07/04 01:18:27, tkent wrote: > IGNORE_EXCEPTION ...
6 years, 5 months ago (2014-07-04 02:06:56 UTC) #12
kangil_
6 years, 5 months ago (2014-07-04 02:59:15 UTC) #13
Message was sent while issue was closed.
I have uploaded follow-up patch at https://codereview.chromium.org/363383004/
PTAL, thanks!

Powered by Google App Engine
This is Rietveld 408576698