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

co19_2 html tests should not use document.registerElement #673

Closed
srujzs opened this issue May 20, 2020 · 11 comments
Closed

co19_2 html tests should not use document.registerElement #673

srujzs opened this issue May 20, 2020 · 11 comments
Assignees
Labels
status-blocked Blocked from making progress by another (referenced) issue

Comments

@srujzs
Copy link

srujzs commented May 20, 2020

document.registerElement (or document.register in the dart:html library) has been deprecated and removed in Chrome 80. In order to replicate the same behavior, window.customElements.define is preferred instead. See https://developer.mozilla.org/en-US/docs/Web/API/Document/registerElement for more details.

This currently affects the following tests:

co19_2/LibTest/html/Element/attributeChanged_A01_t01
co19_2/LibTest/html/Element/enteredView_A01_t01
co19_2/LibTest/html/Element/leftView_A01_t01
co19_2/LibTest/html/Element/tagName_A01_t03
co19_2/LibTest/html/IFrameElement/IFrameElement.created_A01_t01
co19_2/LibTest/html/IFrameElement/tagName_A01_t03

The error looks like:

TypeError: receiver.registerElement is not a function
TypeError: receiver.registerElement is not a function
    at HtmlDocument.registerElement2  org-dartlang-sdk:///sdk_nnbd/lib/html/dart2js/html_dart2js.dart 17723:5
    at registerElement(inlined)       org-dartlang-sdk:///sdk_nnbd/lib/html/dart2js/html_dart2js.dart 10392:5
    at register(inlined)              org-dartlang-sdk:///sdk_nnbd/lib/html/dart2js/html_dart2js.dart 17729:12
@sigmundch
Copy link
Member

We may want to remove some of these tests or switch to run the tests with the web_components polyfill that was used for browsers that didn't implement the API before it was spec-ed.

For context, CustomElements were implemented for Dart when the spec was at version 0.5. It was later noticed that the newer APIs required defining ES6 classes in a way that was not easily compatible with Dart, so we are not fully supporting the new API in Dart at this time.

I am not sure, but I don't believe window.customElements.define will work from Dart at this time.

@sigmundch
Copy link
Member

Here is the issue describing our decision on not adding support for custom-elements v1 (dart-lang/sdk#27445)

@sgrekhov sgrekhov self-assigned this May 21, 2020
@sgrekhov
Copy link
Contributor

sgrekhov commented May 21, 2020

I'm trying to check if window.customElements.define will work or not, but unable to call it in a Dart code. This function signature looks like

void define (
String name,
Object constructor,
[Map options]
)

The second argument is a constructor. But you cannot tear-off a constructor in Dart. Closure also doesn't work

class Foo extends HtmlElement {
  Foo.created() : super.created();
...
  }
}

main() {
  var tag = 'x-foo';
  window.customElements.define(tag, () => new Foo.created());  // Error in JS. 'execute 'define' on 'CustomElementRegistry': The callback provided as parameter 2 is not a function.'
  ...
}

@sigmundch @srujzs , could you, please, advice how to use window.customElements.define in Dart correctly?

@srujzs
Copy link
Author

srujzs commented May 21, 2020

I'll toy around to see if there's an alternative to this or if we should just remove tests if the functionality doesn't make sense anymore like @sigmundch suggested. I'll have to get back to you on this one.

@sgrekhov
Copy link
Contributor

@srujzs any update on this?

@srujzs
Copy link
Author

srujzs commented Dec 24, 2020

I haven't spent much time on this, but both you and @sigmundch is right that we can't supply a constructor here for define. Supplying the Dart custom type doesn't work either. I'm assuming you're seeing the error with your code sample because it's not wrapped with allowInterop, but doing so gives me a different error further downstream that I'll have to continue investigating.

@srujzs
Copy link
Author

srujzs commented Dec 28, 2020

Spent a little more time on this, but I don't think there's any way to use define with dart:html. Supplying anything besides a constructor won't/shouldn't work regardless of if the callback allows interop, and since we can't do that, define won't work. This is unfortunate since with registerElement being deprecated, there is no way forward with registering a custom element. For lack of a better option here, these tests should be removed, or at least the parts which rely on registering a custom element removed.

@sgrekhov
Copy link
Contributor

@srujzs thank you! I'll rewrite these tests

@sgrekhov
Copy link
Contributor

This is unfortunate since with registerElement being deprecated, there is no way forward with registering a custom element.

@srujzs I don't see that document.registerElement() is deprecated in the current Dart API. I do see that document.register() is deprecated, but not registerElement(). May be simply use registerElement() instead of register()?

@srujzs
Copy link
Author

srujzs commented Dec 30, 2020

I think that's just stale documentation at this point and dart:html hasn't been updated to reflect the deprecation changes since Chrome 80. The underlying registerElement native function no longer exists in any major browser: https://developer.mozilla.org/en-US/docs/Web/API/Document/registerElement. Therefore, neither register (which uses registerElement) nor registerElement/registerElement2 will work.

For the above tests, if the behavior that is being tested does not need to rely on custom elements to work (I can't really tell from a quick inspection), it may be still useful to have those tests but use something other than custom elements. I suppose one other alternative is using eval to define the custom element if we really do need it, but I'd stick with rewriting the tests that don't need custom elements for now. There might be several gotchas using eval to operate on custom elements so it's better to wait to do that. I see at least one of our own tests in tests/lib/html that test custom elements, so if we do refactor that one to use eval, it also might be easier for you to wait until then to refactor the co19 tests that need to use custom elements.

@sgrekhov sgrekhov added the status-blocked Blocked from making progress by another (referenced) issue label Aug 26, 2021
eernstg pushed a commit that referenced this issue Jul 29, 2022
Authored by @sgrekhov. Removes two tests that do not serve the stated purpose.
eernstg pushed a commit that referenced this issue Jul 29, 2022
Authored by @sgrekhov.

Two IFrame tests that don't actually test IFrame removed.
sgrekhov added a commit to sgrekhov/co19 that referenced this issue Aug 4, 2022
…art-lang#1382)

Authored by @sgrekhov. Removes two tests that do not serve the stated purpose.
copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Aug 11, 2022
2022-08-04 sgrekhov22@gmail.com dart-lang/co19#993. FFI tests refactored and improved (dart-lang/co19#1387)
2022-08-04 sgrekhov22@gmail.com dart-lang/co19#993. [FFI] Missed tests added for some new types (dart-lang/co19#1386)
2022-08-03 sgrekhov22@gmail.com Fixes dart-lang/co19#1129. Don't expect any proxy routine in case of DIRECT connection (dart-lang/co19#1383)
2022-08-03 sgrekhov22@gmail.com dart-lang/co19#993. [FFI] Missed tests added for some new types (dart-lang/co19#1384)
2022-07-29 sgrekhov22@gmail.com dart-lang/co19#673. IFrame tests that don't actually test IFrame removed (dart-lang/co19#1382)
2022-07-29 sgrekhov22@gmail.com dart-lang/co19#1363. Some delays removed or reduced (dart-lang/co19#1380)
2022-07-29 sgrekhov22@gmail.com dart-lang/co19#1363 Async `for-in` test updated according to the current specification (dart-lang/co19#1379)
2022-07-27 sgrekhov22@gmail.com dart-lang/co19#1363. Yield each tests updated to the current specification (dart-lang/co19#1374)
2022-07-27 sgrekhov22@gmail.com Fixes dart-lang/co19#1375. Add test for final variable in a for-in loop (dart-lang/co19#1378)
2022-07-25 sgrekhov22@gmail.com dart-lang/co19#1363. Tests for yield updated to the current specification (dart-lang/co19#1370)

Change-Id: Ie92f4b2157284777177012320883247411e214fa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/254140
Reviewed-by: Erik Ernst <eernst@google.com>
Commit-Queue: Alexander Thomas <athom@google.com>
@sgrekhov
Copy link
Contributor

sgrekhov commented Sep 1, 2022

All of these tests use deprecated API and were removed as part of #1405. Closed

@sgrekhov sgrekhov closed this as completed Sep 1, 2022
copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Sep 30, 2022
2022-09-30 sgrekhov22@gmail.com Fixes dart-lang/co19#1478. Failures of the new co19_2 roll fixed. (dart-lang/co19#1480)
2022-09-28 sgrekhov22@gmail.com dart-lang/co19#195. RawDatagramSocket tests fixed (dart-lang/co19#1465)
2022-08-31 sgrekhov22@gmail.com dart-lang/co19#1405. Don't use deprecated API in co19_2 tests (dart-lang/co19#1409)
2022-08-31 sgrekhov22@gmail.com dart-lang/co19#1405. co19_2. BytesBuilder tests moved from dart:io to dart:typed_data (dart-lang/co19#1411)
2022-08-23 sgrekhov22@gmail.com dart-lang/co19#1394. Add missing compile-error (dart-lang/co19#1397)
2022-08-03 sgrekhov22@gmail.com dart-lang/co19#1129. In case of DIRECT connection don't expect any proxy routine called (dart-lang/co19#1385)
2022-07-29 sgrekhov22@gmail.com dart-lang/co19#673. IFrame tests that don't actually test IFrame removed (dart-lang/co19#1381)
2022-07-27 sgrekhov22@gmail.com Fixes dart-lang/co19#1376. Broken co19_2 test fixed (dart-lang/co19#1377)
2022-07-12 sgrekhov22@gmail.com Fixes dart-lang/co19#1356. Add analyzer errors on references to not included library parts (dart-lang/co19#1359)
2022-07-05 sgrekhov22@gmail.com Fixes dart-lang/co19#1309. Update error expectations according to the current behavior (dart-lang/co19#1351)
2022-07-04 sgrekhov22@gmail.com Fixes dart-lang/co19#1343. Use correct 'part' and 'part of' directives (dart-lang/co19#1350)
2022-07-01 sgrekhov22@gmail.com Fixes dart-lang/co19#1313. Pre-nnbd tests for focus_A01_t01 and blur_A01_t01 updated to their version in master (dart-lang/co19#1345)

Change-Id: Iec8355f25b2c87b3dd83d84d2bab95812bbd0686
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/262204
Reviewed-by: Alexander Thomas <athom@google.com>
Commit-Queue: Alexander Thomas <athom@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status-blocked Blocked from making progress by another (referenced) issue
Projects
None yet
Development

No branches or pull requests

3 participants