Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

strange dart2js+unchecked+shadowdom+htmldocument bug #14720

Closed
jmesserly opened this issue Nov 1, 2013 · 17 comments
Closed

strange dart2js+unchecked+shadowdom+htmldocument bug #14720

jmesserly opened this issue Nov 1, 2013 · 17 comments
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures
Milestone
1.2

Comments

@jmesserly
Copy link

see notes here: https://codereview.chromium.org/54983011/

adding import 'dart:mirrors' to a polymer test is enough to cause the failure. The failure happens after the test has already passed, when the unittest framework is trying to replace the innerHtml with the results.

Tentatively, I'm guessing this is related to ShadowDOM. But it might reproduce with just dart2js mirrors & HTML.
@jmesserly
Copy link
Author

what seems to be happening:

  • setInnerHtml creates a document
  • it tries to append <base> to the <head>
  • interceptor gets "Document" instead of "HtmlDocument"
  • so the document.head fails

It's not clear why we're getting the wrong interceptor, and why it only happens sometimes.

@jmesserly
Copy link
Author

even stranger: according to Siggi this happens more when he switches shadow_dom polyfill (and custom elements) to .min.js

@kasperl
Copy link

kasperl commented Nov 6, 2013

Maybe Stephen has some insights?

cc @rakudrama.

@sigmundch
Copy link
Member

Marked this as blocking #14326.

@DartBot
Copy link

DartBot commented Nov 13, 2013

This comment was originally written by @rbishop-bah

A snippet from the wild that may or may not help:

  createFragment$3$treeSanitizer$validator: function(receiver, html, treeSanitizer, validator) {
    var t1, t2, t3, base, contextElement, fragment;
    if (treeSanitizer == null) {
      if (validator == null) {
        if ($.Element__defaultValidator == null) {
          t1 = [];
          H.setRuntimeTypeInfo(t1, [W.NodeValidator]);
          t1 = new W.NodeValidatorBuilder(t1);
          t2 = t1._validators;
          t3 = J.getInterceptor$ax(t2);
          t3.add$1(t2, W._Html5NodeValidator$(null));
          t3.add$1(t2, W._TemplatingNodeValidator$());
          $.Element__defaultValidator = t1;
        }
        validator = $.Element__defaultValidator;
      }
      t1 = $.Element__defaultSanitizer;
      if (t1 == null)
        $.Element__defaultSanitizer = new W._ValidatingTreeSanitizer(validator);
      else
        t1.set$validator(validator);
      treeSanitizer = $.Element__defaultSanitizer;
    } else if (validator != null)
      throw H.wrapException(new P.ArgumentError("validator can only be passed if treeSanitizer is null"));
    if ($.Element__parseDocument == null) {
      $.Element__parseDocument = document.implementation.createHTMLDocument(""); <===== THIS CREATES A Document AND NOT AN HtmlDocument
      $.Element__parseRange = J.createRange$0$x($.Element__parseDocument);
      base = J.createElement$1$x($.Element__parseDocument, "base");
      J.set$href$x(base, document.baseURI);
      J.append$1$x(J.get$head$x($.Element__parseDocument), base);
    }
    t1 = $.Element__parseDocument;
    if (!!this.$isBodyElement)
      contextElement = J.get$body$x(t1);
    else {
      contextElement = J.createElement$1$x(t1, receiver.tagName);
      J.append$1$x(J.get$body$x($.Element__parseDocument), contextElement); &lt;====== THIS CHOKES BECAUSE $.Element__parseDocument IS NOT AN HtmlDocument
    }

@sigmundch
Copy link
Member

Added this to the M9 milestone.

@clayberg
Copy link

Removed this from the M9 milestone.
Added this to the 1.1 milestone.

@DartBot
Copy link

DartBot commented Nov 21, 2013

This comment was originally written by @rbishop-bah

See discussion:
https://groups.google.com/a/dartlang.org/forum/?fromgroups=#!topic/web-ui/GOZyLkZuq4I

@jmesserly
Copy link
Author

Chatted with Stephen. Our thought here is the interceptor lookup is probably broken in Shadow DOM polyfill. The reason it only reproduces in checked mode+mirrors:

* mirrors cause more things to be dynamically dispatched

  • checked mode causes more types to be retained, causing Document <--> HTMLDocument to be treated as different.

@keertip
Copy link
Contributor

keertip commented Nov 21, 2013

Marked this as blocking #15253.

@jmesserly
Copy link
Author

Removed Priority-Unassigned label.
Added Priority-High label.

@DartBot
Copy link

DartBot commented Nov 22, 2013

This comment was originally written by @rbishop-bah

FYI, refactoring my code to use dart:js exclusively instead of package js circumvented this issue for me.

@sigmundch
Copy link
Member

Unmarked this as blocking #15253.

@sigmundch
Copy link
Member

Some updates about this bug.

Stephen has an idea about how to fix this on the dart2js side of things. He thinks it might be better to fix it there instead of the shadow_dom polyfill since this kind of issue can arise again in the future with other libraries that do similar things in JS.

I tried to make the example smaller, and here is a simplified example that can be used to reproduce the problem without depending on polymer.

a.dart:
  import 'dart:html';
  main() {
    document.body.query('div').innerHtml = "bar";
  }

a.html:
  <body><div></div>
  <script>window.__forceShadowDomPolyfill = true;</script>
  <script src="packages/shadow_dom/shadow_dom.debug.js"></script>
  <script src="a.dart.js"></script>

Where a.dart.js is generated by calling:

  dart2js --disable-type-inference a.dart -o a.dart.js

Adding the '--disable-type-inference' flag makes this error trigger consistently.

Applying Stephen's patch in the generated code (https://codereview.chromium.org/86993002) gets rid of the error.

@sigmundch
Copy link
Member

Removed this from the 1.1 milestone.
Added this to the 1.2 milestone.

@jmesserly
Copy link
Author

Stephen fixed this a while ago

Added Fixed label.

@anders-sandholm
Copy link
Contributor

Removed Library-ShadowDOM label.
Added Pkg-ShadowDOM label.

@jmesserly jmesserly added Type-Defect P1 A high priority bug; for example, a single project is unusable or has many test failures labels Feb 6, 2014
@jmesserly jmesserly added this to the 1.2 milestone Feb 6, 2014
This was referenced Jun 3, 2015
Closed
Closed
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures
Projects
None yet
Milestone
1.2
Development

No branches or pull requests
7 participants
@jmesserly @sigmundch @kasperl @keertip @anders-sandholm @clayberg @DartBot