Last Comment Bug 613662 - Implement insertAdjacentHTML
: Implement insertAdjacentHTML
Status: RESOLVED FIXED
[wanted2.1?][inbound]
: dev-doc-complete, html5
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal with 2 votes (vote)
: mozilla8
Assigned To: Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01)
:
Mentors:
http://html5.org/specs/dom-parsing.ht...
Depends on: 676808 563322 675621 681052
Blocks: domparsing
  Show dependency treegraph
 
Reported: 2010-11-19 16:28 PST by Jonas Sicking (:sicking) PTO Until July 5th
Modified: 2012-12-02 17:49 PST (History)
23 users (show)
hsivonen: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Implement insertAdjacentHTML (6.59 KB, patch)
2011-03-31 05:18 PDT, Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01)
no flags Details | Diff | Splinter Review
Tests for insertAdjacentHTML (11.24 KB, patch)
2011-03-31 05:18 PDT, Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01)
no flags Details | Diff | Splinter Review
Implement insertAdjacentHTML, v2 (7.00 KB, patch)
2011-04-06 02:27 PDT, Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01)
bzbarsky: review+
Details | Diff | Splinter Review
Tests for insertAdjacentHTML, v2 (11.52 KB, patch)
2011-04-06 02:27 PDT, Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01)
bzbarsky: review+
Details | Diff | Splinter Review
Implemention difference from v1 (1.81 KB, patch)
2011-04-06 02:28 PDT, Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01)
no flags Details | Diff | Splinter Review
Test difference from v1 (996 bytes, patch)
2011-04-06 02:28 PDT, Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01)
no flags Details | Diff | Splinter Review
Disable the part of the tests that needs bug 676808 to be fixed (1.76 KB, patch)
2011-08-05 06:00 PDT, Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01)
bugs: review+
Details | Diff | Splinter Review

Description Jonas Sicking (:sicking) PTO Until July 5th 2010-11-19 16:28:26 PST
It's now part of HTML5, and might be help us get people off of doing

foo.innerHTML += "...";

http://dev.w3.org/html5/spec/Overview.html#insertadjacenthtml

It's too late for FF4, but there's no reason not to do it ASAP after.

Ms2ger expressed some interest in doing it (or rather, didn't violently oppose it when I suggested it)
Comment 1 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2011-03-30 07:07:43 PDT
Looking at this per discussion with bz.
Comment 2 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2011-03-31 05:18:35 PDT
Created attachment 523282 [details] [diff] [review]
Implement insertAdjacentHTML
Comment 3 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2011-03-31 05:18:56 PDT
Created attachment 523283 [details] [diff] [review]
Tests for insertAdjacentHTML
Comment 4 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2011-04-06 01:53:26 PDT
Need to revise the patch to take into account http://www.w3.org/Bugs/Public/show_bug.cgi?id=12434

The current patch behaves like IE on the non-optimized code path and per current spec on the optimized code path, so the patch is buggy either way.
Comment 5 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2011-04-06 02:27:12 PDT
Created attachment 524157 [details] [diff] [review]
Implement insertAdjacentHTML, v2
Comment 6 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2011-04-06 02:27:56 PDT
Created attachment 524158 [details] [diff] [review]
Tests for insertAdjacentHTML, v2
Comment 7 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2011-04-06 02:28:20 PDT
Created attachment 524159 [details] [diff] [review]
Implemention difference from v1
Comment 8 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2011-04-06 02:28:41 PDT
Created attachment 524160 [details] [diff] [review]
Test difference from v1
Comment 9 Boris Zbarsky [:bz] 2011-04-21 09:48:08 PDT
Comment on attachment 524157 [details] [diff] [review]
Implement insertAdjacentHTML, v2

Please fix the checkin comment's try stuff before pushing.

>+    // HTML5 parser has notified, but not fired mutation events.

This is duplicating code in SetInnerHTML.  Can we factor it out into a helper instead?

You need to rev the nsIDOMNSHTMLElement.idl iid.

I assume the createContextualFragment call will need to be merged with the patch for bug 596182?

r=me with those nits addressed.  Sorry for the lag here....
Comment 10 Boris Zbarsky [:bz] 2011-04-21 09:48:33 PDT
Comment on attachment 524158 [details] [diff] [review]
Tests for insertAdjacentHTML, v2

r=me
Comment 11 Jonas Sicking (:sicking) PTO Until July 5th 2011-05-11 12:59:45 PDT
Any chance this can make FF6? Branching is in a week.
Comment 12 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2011-05-11 13:33:39 PDT
(In reply to comment #11)
> Any chance this can make FF6? Branching is in a week.

Unlikely. :-(

This depends on stuff that happens to depend on the patch for bug 482909. In retrospect, it would have been a better idea to write the patches in a different order to avoid depending on bug 482909, but if I tried to reorder them now, I'd end up invalidating a bunch of reviews I've already obtained. Also, I'm generally hesitant to reorder the patches in haste at this point.

Also, even if bug 482909 got review before the branching, it would be at least against the spirit of the new rules to dump the queue on central right before a release train leaves.
Comment 13 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2011-08-01 01:04:04 PDT
Landed in inbound:
http://hg.mozilla.org/integration/mozilla-inbound/rev/047bd50613ab
http://hg.mozilla.org/integration/mozilla-inbound/rev/7e84bd591246
Comment 14 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2011-08-01 02:55:46 PDT
...and backed out the test. Bug 650493 had landed in the meantime and had changed the exact mutation events that fire. The mutation event expectations is the test case weren't based on a spec but based on what the Gecko behavior was when I wrote the test.
Comment 15 Marco Bonardo [::mak] 2011-08-01 08:09:08 PDT
http://hg.mozilla.org/mozilla-central/rev/047bd50613ab

leaving open since the tests have to be relanded yet
Comment 16 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2011-08-05 06:00:29 PDT
Created attachment 551019 [details] [diff] [review]
Disable the part of the tests that needs bug 676808 to be fixed

This patch applies over attachment 524158 [details] [diff] [review]. This patch comments out the part of the tests that fails until bug 676808 is fixed. I suggest relanding the tests plus this patch.
Comment 17 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2011-08-05 06:03:42 PDT
Comment on attachment 551019 [details] [diff] [review]
Disable the part of the tests that needs bug 676808 to be fixed

Moving review request to optimize timezone situation.
Comment 18 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2011-08-05 09:49:26 PDT
Thanks. Pushed:
http://hg.mozilla.org/integration/mozilla-inbound/rev/d900907a1038
http://hg.mozilla.org/integration/mozilla-inbound/rev/fc40a7ae42ca
Comment 19 Marco Bonardo [::mak] 2011-08-06 02:49:05 PDT
http://hg.mozilla.org/mozilla-central/rev/d900907a1038
http://hg.mozilla.org/mozilla-central/rev/fc40a7ae42ca

looks like work here has finished, resolving.
Comment 20 Steffen Wilberg 2011-11-16 03:38:21 PST
Just for reference:
The spec has moved from HTML5 to DOM Parsing:
http://html5.org/specs/dom-parsing.html#insertadjacenthtml()

See http://www.w3.org/Bugs/Public/show_bug.cgi?id=11204

https://developer.mozilla.org/en/DOM/element.insertAdjacentHTML already links to the new location.

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