Last Comment Bug 851916 - createHTMLDocument() should work with no arguments
: createHTMLDocument() should work with no arguments
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: mozilla23
Assigned To: Aryeh Gregor (:ayg) (next working March 28-April 26)
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks: 856629
  Show dependency treegraph
 
Reported: 2013-03-17 08:24 PDT by Aryeh Gregor (:ayg) (next working March 28-April 26)
Modified: 2013-04-26 13:33 PDT (History)
5 users (show)
ayg: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (6.02 KB, patch)
2013-03-19 07:51 PDT, Aryeh Gregor (:ayg) (next working March 28-April 26)
bzbarsky: review+
Details | Diff | Splinter Review
Sync (parts of) dom/imptests/ (144.80 KB, patch)
2013-03-21 05:39 PDT, Aryeh Gregor (:ayg) (next working March 28-April 26)
no flags Details | Diff | Splinter Review
Sync (parts of) dom/imptests/, v2 (103.88 KB, patch)
2013-03-21 08:05 PDT, Aryeh Gregor (:ayg) (next working March 28-April 26)
Ms2ger: review+
Details | Diff | Splinter Review
Disable on B2G (2.82 KB, patch)
2013-04-07 10:36 PDT, :Ms2ger (⌚ UTC+1/+2)
ayg: review+
Details | Diff | Splinter Review

Description Aryeh Gregor (:ayg) (next working March 28-April 26) 2013-03-17 08:24:35 PDT
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.
Comment 1 :Ms2ger (⌚ UTC+1/+2) 2013-03-17 08:37:54 PDT
Spec made this optional in <https://github.com/whatwg/dom/commit/ce4ffbdaf61caddb6851c1e1fe3f87bbc25ea588>.
Comment 2 Boris Zbarsky [:bz] (still a bit busy) 2013-03-18 14:19:22 PDT
The WebKit behavior seems fine to me.
Comment 3 Aryeh Gregor (:ayg) (next working March 28-April 26) 2013-03-19 07:51:34 PDT
Created attachment 726662 [details] [diff] [review]
Patch

Try: https://tbpl.mozilla.org/?tree=Try&rev=b26318c9d195
Comment 4 Boris Zbarsky [:bz] (still a bit busy) 2013-03-19 10:26:27 PDT
Comment on attachment 726662 [details] [diff] [review]
Patch

>+  if (!aTitle.IsVoid()) {

  if (!DOMStringIsNull(aTitle)) {

please.

r=me with that, I guess.
Comment 5 Aryeh Gregor (:ayg) (next working March 28-April 26) 2013-03-21 05:39:50 PDT
Created attachment 727640 [details] [diff] [review]
Sync (parts of) dom/imptests/

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.
Comment 6 Aryeh Gregor (:ayg) (next working March 28-April 26) 2013-03-21 05:45:52 PDT
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
Comment 7 :Ms2ger (⌚ UTC+1/+2) 2013-03-21 05:59:17 PDT
I'm afraid you'll have to reapply <http://hg.mozilla.org/mozilla-central/rev/b35c960626b5>.
Comment 8 Aryeh Gregor (:ayg) (next working March 28-April 26) 2013-03-21 08:05:29 PDT
Created attachment 727684 [details] [diff] [review]
Sync (parts of) dom/imptests/, v2

Okay.  I'm assuming that doesn't need a new try run, unless the current one fails.
Comment 9 :Ms2ger (⌚ UTC+1/+2) 2013-03-21 10:57:25 PDT
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.)
Comment 10 Aryeh Gregor (:ayg) (next working March 28-April 26) 2013-04-02 08:49:51 PDT
Lost try runs in the reset.  New ones:

https://tbpl.mozilla.org/?tree=Try&rev=ae1ce91f3291
https://tbpl.mozilla.org/?tree=Try&rev=a65b811eba84
Comment 11 Aryeh Gregor (:ayg) (next working March 28-April 26) 2013-04-04 05:10:02 PDT
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.
Comment 12 Ryan VanderMeulen [:RyanVM] 2013-04-04 09:05:08 PDT
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.
Comment 13 Ryan VanderMeulen [:RyanVM] 2013-04-04 09:32:44 PDT
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
Comment 14 :Ms2ger (⌚ UTC+1/+2) 2013-04-07 10:36:11 PDT
Created attachment 734396 [details] [diff] [review]
Disable on B2G

This looks happy enough:

https://tbpl.mozilla.org/?tree=Try&rev=d9318292e5ff
Comment 15 Aryeh Gregor (:ayg) (next working March 28-April 26) 2013-04-08 03:44:27 PDT
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?
Comment 16 :Ms2ger (⌚ UTC+1/+2) 2013-04-08 04:07:10 PDT
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?

Note You need to log in before you can comment on or make changes to this bug.