Closed Bug 1226398 Opened 4 years ago Closed 3 years ago

Stop using legacy generators in dom/indexedDB/test

Categories

(Core :: DOM: Workers, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: emk, Assigned: arai)

References

Details

Attachments

(2 files)

No description provided.
should be fixed by bug 1321218.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1321218
apparently there's enough number of overlooked consumers.
will fix here.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Assignee: nobody → arai.unmht
Status: REOPENED → ASSIGNED
Blocks: 968038
No longer blocks: 1083482
just changed legacy generator to ES6 generator.
Attachment #8835718 - Flags: review?(jaws)
Comment on attachment 8835718 [details] [diff] [review]
Remove remaining legacy generator from dom/indexedDB/test/.

Review of attachment 8835718 [details] [diff] [review]:
-----------------------------------------------------------------

Can we get an eslint rule to enforce this? Look at the patch on bug 1329182 to get an example of how an eslint rule can be created. Feel free to join #eslint if you want to ask questions.
this is just an invalid syntax per spec, so applying eslint with standard configuration should catch it.

there's exceptional case, that is yield without operand, in non strict mode:
  function f() { yield; }
it's currently parsed as legacy generator in SpiderMonkey,
but it's also a valid syntax per spec, since it's just an identifier outside of generator.
I don't think it worth detecting that case, as I don't see that case in all files I touched this time (including other bugs)

then, currently dom/ is ignored in eslint.
I'll change it to check dom/indexedDB.
changed .eslintignore to include dom/indexedDB.
also, to pass the test, changed conditional catch to standard syntax.
Attachment #8835756 - Flags: review?(jaws)
Comment on attachment 8835756 [details] [diff] [review]
Part 2: Check dom/indexedDB/ in eslint.

Review of attachment 8835756 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, this is great!
Attachment #8835756 - Flags: review?(jaws) → review+
https://hg.mozilla.org/mozilla-central/rev/f248ec7b14ab
https://hg.mozilla.org/mozilla-central/rev/0ec46eb0c0ed
Status: ASSIGNED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Again, test_getFileId.html still has a legacy generator. ESLint did not catch this because... the type attribute has a versioned parameter! Apparently ESLint ignores <script>s if the type attribute contains a version parameter.
Flags: needinfo?(arai.unmht)
(In reply to Masatoshi Kimura [:emk] from comment #10)
> Again, test_getFileId.html still has a legacy generator. ESLint did not
> catch this because... the type attribute has a versioned parameter!
> Apparently ESLint ignores <script>s if the type attribute contains a version
> parameter.

thanks.
apparently the test file is not used.
I don't see any reference for "test_getFileId.html" in tree, and also it's not hit in try run.
Flags: needinfo?(arai.unmht)
Hm, bug 994190 removed test_getFileId.html from mochitest.ini, but it didn't remove the file itself. Moreover, I could not find the explanation why the test was removed.
You need to log in before you can comment on or make changes to this bug.