Closed Bug 851916 Opened 12 years ago Closed 12 years ago

createHTMLDocument() should work with no arguments

Categories

(Core :: DOM: Core & HTML, enhancement)

Product:
Core
Component:
DOM: Core & HTML
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla23

People

(Reporter: ayg, Assigned: ayg)

References

Details

(Keywords: dev-doc-complete)

Attachments

(3 files, 1 obsolete file)

Patch
(deleted), patch
bzbarsky
: review+
Details | Diff | Splinter Review
Sync (parts of) dom/imptests/, v2
(deleted), patch
Ms2ger
: review+
Details | Diff | Splinter Review
Disable on B2G
(deleted), patch
ayg
: review+
Details | Diff | Splinter Review
The spec says that if no title argument is provided, the <head> should be empty, with no <title> element: http://dom.spec.whatwg.org/#dom-domimplementation-createhtmldocument It seems this is how WebKit works. The versions of IE, Firefox, and Opera I checked throw if you pass no arguments. This is tested by: http://w3c-test.org/webapps/DOMCore/tests/submissions/Ms2ger/DOMImplementation-createHTMLDocument.html Boris, should we change to match the spec or ask for the spec to change? The spec behavior seems slightly more useful to me -- I don't see why <title> should be required.
Flags: needinfo?(bzbarsky)
The WebKit behavior seems fine to me.
Flags: needinfo?(bzbarsky)
Comment on attachment 726662 [details] [diff] [review] Patch >+ if (!aTitle.IsVoid()) { if (!DOMStringIsNull(aTitle)) { please. r=me with that, I guess.
Attachment #726662 - Flags: review?(bzbarsky) → review+
Attached patch Sync (parts of) dom/imptests/ (obsolete) (deleted) — — Splinter Review
Needed to fix regressions in dom/imptests/, since the IDL we have is from an outdated spec version. Best-guess reasons for new passes/failures: * DOMError interface constructor: Spec changed to add a constructor. * isTrusted: Spec changed to make it Unforgeable. * initCustomEvent: Newly added function in spec. * EventListener: idlharness now has support for callback interfaces. * Mutation stuff: Rewritten in spec. * children, first/lastElementChild, childElementCount: Moved from Element to new ParentNode interface. * prepend, append, before, after, replace: Now uncommented in the copied IDL. * createHTMLDocument: Spec changed to match WebKit, about to be fixed in this bug. * previousElementSibling, nextElementSibling: Moved from Element to new ChildNode interface. * NodeFilter: idlharness now has support for callback interfaces. * DOMStringList: Interface removed from spec. * NodeList: Spec updated to make it an ArrayClass. * test_*-remove.html: Tests seem to have been broken before, fixed now. * test_Document-getElementsByTagName.html: Seems to be rewritten. * test_Range-*: Updated for no-opping detach(). So all looks okay to me.
Attachment #727640 - Flags: review?(Ms2ger)
Try for sync patch: https://tbpl.mozilla.org/?tree=Try&rev=6a1d31712203 Try for real patch on top of sync patch: https://tbpl.mozilla.org/?tree=Try&rev=77896f02b36b
Status: NEW → ASSIGNED
Okay. I'm assuming that doesn't need a new try run, unless the current one fails.
Attachment #727640 - Attachment is obsolete: true
Attachment #727640 - Flags: review?(Ms2ger)
Attachment #727684 - Flags: review?(Ms2ger)
Comment on attachment 727684 [details] [diff] [review] Sync (parts of) dom/imptests/, v2 Review of attachment 727684 [details] [diff] [review]: ----------------------------------------------------------------- All comments are just me talking to myself. Thanks for doing this! ::: dom/imptests/idlharness.js @@ +1053,5 @@ > //@{ > { > test(function() > { > + // This function tests WebIDL as of 2012-11-28. And at that time, I was planning to review the entire file "soon"... *sigh* ::: dom/imptests/testharness.js @@ +1404,5 @@ > Tests.prototype.notify_complete = function() > { > clearTimeout(this.timeout_id); > var this_obj = this; > + var tests = map(this_obj.tests, (AFAICT, there's no map() in scope... I wonder if this code ever runs.) ::: dom/imptests/webapps.mozbuild @@ +1,1 @@ > +# THIS FILE IS AUTOGENERATED BY importTestSuites.py - DO NOT EDIT (I'm pretty sure that's not what it's called.)
Attachment #727684 - Flags: review?(Ms2ger) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/abbc05319449 https://hg.mozilla.org/integration/mozilla-inbound/rev/715fce49a07b The first changeset (sync of test suite) cherry-picks this patch in addition to the patch posted here: https://github.com/jgraham/testharness.js/commit/1a0dfc26cc07faff88cc9f2918f5a42d41432b71 suggested by Ms2ger to fix the try failures.
Flags: in-testsuite+
Backed out for B2G mochitest-2 perma-orange. https://hg.mozilla.org/integration/mozilla-inbound/rev/d26881860f30 https://tbpl.mozilla.org/php/getParsedLog.php?id=21431843&tree=Mozilla-Inbound 07:26:17 INFO - 27104 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/imptests/editing/selecttest/test_addRange.html | Test timed out. 07:26:17 INFO - 27105 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/imptests/editing/selecttest/test_addRange.html | Test runner timed us out. 07:26:17 INFO - 27107 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/imptests/editing/selecttest/test_addRange.html | Test timed out. 07:26:17 INFO - 27108 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/imptests/editing/selecttest/test_addRange.html | Test runner timed us out. These tests are disabled on Android, FWIW. Up to you to decide how you want to proceed.
Also hit this on OSX: https://tbpl.mozilla.org/php/getParsedLog.php?id=21434788&tree=Mozilla-Inbound 09:12:28 INFO - 7021 INFO TEST-START | /tests/dom/imptests/html/html/browsers/the-window-object/test_window-named-properties.html 09:12:28 INFO - ++DOMWINDOW == 56 (0x16001cb10) [serial = 2110] [outer = 0x12e0ab730] 09:12:28 INFO - ++DOCSHELL 0x12f867420 == 12 [id = 582] 09:12:28 INFO - ++DOMWINDOW == 57 (0x178e9fa20) [serial = 2111] [outer = 0x0] 09:12:28 INFO - ++DOCSHELL 0x148e9fd20 == 13 [id = 583] 09:12:28 INFO - ++DOMWINDOW == 58 (0x153bd1950) [serial = 2112] [outer = 0x0] 09:12:28 INFO - [Parent 390] WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && subjPrincipal) failed: file ../../../docshell/base/nsDocShell.cpp, line 8386 09:12:28 INFO - [Parent 390] WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && subjPrincipal) failed: file ../../../docshell/base/nsDocShell.cpp, line 8386 09:12:28 INFO - [Parent 390] WARNING: Subdocument container has no frame: file ../../../layout/base/nsDocumentViewer.cpp, line 2388 09:12:28 INFO - ++DOMWINDOW == 59 (0x160097300) [serial = 2113] [outer = 0x178e9fa20] 09:12:28 INFO - ++DOMWINDOW == 60 (0x165d9bcb0) [serial = 2114] [outer = 0x153bd1950] 09:12:32 INFO - 7022 INFO TEST-PASS | /tests/dom/imptests/html/html/browsers/the-window-object/test_window-named-properties.html | Elided 3 passes or known failures. 09:12:32 INFO - 7023 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/imptests/html/html/browsers/the-window-object/test_window-named-properties.html | Dynamic name; Test timed out
Attached patch Disable on B2G (deleted) — — Splinter Review
This looks happy enough: https://tbpl.mozilla.org/?tree=Try&rev=d9318292e5ff
Attachment #734396 - Flags: review?(ayg)
Comment on attachment 734396 [details] [diff] [review] Disable on B2G Review of attachment 734396 [details] [diff] [review]: ----------------------------------------------------------------- What's this doing, disabling the tests in question? If so, LGTM. Unlikely that platform-specific code is going to behave differently for these tests. Why did you only fill in one of the values with the bug number, though? Shouldn't it be all or none?
Attachment #734396 - Flags: review?(ayg) → review+
I copied the comments from the android.json in the same folder; doesn't matter to me what they are. Are you going to reland or should I?
https://hg.mozilla.org/mozilla-central/rev/f2963140e620 https://hg.mozilla.org/mozilla-central/rev/d6ac13733d48
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: