Last Comment Bug 887320 - SyntaxError: yield without a value is deprecated, and illegal in ES6 (use 'yield undefined' instead) @ chrome://browser/content/places/controller.js:939
: SyntaxError: yield without a value is deprecated, and illegal in ES6 (use 'yi...
Status: RESOLVED FIXED
[good-first-bug][mentor=mak][lang=js]
:
Product: Firefox
Classification: Client Software
Component: Bookmarks & History (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 25
Assigned To: Sushant Hiray [:sushant]
:
: Marco Bonardo [::mak]
Mentors:
: 889527 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-06-26 09:33 PDT by Jared Wein [:jaws] (please needinfo? me)
Modified: 2013-07-03 11:36 PDT (History)
5 users (show)
mak77: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1: Changed yield to yield undefined (1004 bytes, patch)
2013-06-27 12:01 PDT, Sushant Hiray [:sushant]
mak77: review+
Details | Diff | Splinter Review
Bug:887320 Changed deprecated yield to yeild undefined (2.49 KB, patch)
2013-06-27 12:44 PDT, Sushant Hiray [:sushant]
no flags Details | Diff | Splinter Review
Bug:887320 r=mak {Changed yield to yield undefined} (2.49 KB, patch)
2013-07-02 10:53 PDT, Sushant Hiray [:sushant]
hiraysushant: review+
hiraysushant: checkin+
Details | Diff | Splinter Review
Bug:887320 r=mak {Changed yield to yield undefined} (2.51 KB, patch)
2013-07-02 11:01 PDT, Sushant Hiray [:sushant]
hiraysushant: review+
Details | Diff | Splinter Review

Description Jared Wein [:jaws] (please needinfo? me) 2013-06-26 09:33:47 PDT
SyntaxError: yield without a value is deprecated, and illegal in ES6 (use 'yield undefined' instead) @ chrome://browser/content/places/controller.js:939

See this code block,
929     // Do removal in chunks to give some breath to main-thread.
930     function pagesChunkGenerator(aURIs) {
931       while (aURIs.length) {
932         let URIslice = aURIs.splice(0, REMOVE_PAGES_CHUNKLEN);
933         PlacesUtils.bhistory.removePages(URIslice, URIslice.length);
934         Services.tm.mainThread.dispatch(function() {
935           try {
936             gen.next();
937           } catch (ex if ex instanceof StopIteration) {}
938         }, Ci.nsIThread.DISPATCH_NORMAL); 
939         yield;
940       }

http://hg.mozilla.org/mozilla-central/annotate/c3598b276048/browser/components/places/content/controller.js#l939
Comment 1 Sushant Hiray [:sushant] 2013-06-26 13:31:34 PDT
Hey Marco!
I guess this bug is trivial to start. Could you tell me how do I go about cloning the repository?
Comment 2 Sushant Hiray [:sushant] 2013-06-27 12:01:54 PDT
Created attachment 768499 [details] [diff] [review]
v1: Changed yield to yield undefined

Is this sufficient or do I need to change anything else as well?
Regards,
Sushant
Comment 3 Marco Bonardo [::mak] 2013-06-27 12:14:14 PDT
Comment on attachment 768499 [details] [diff] [review]
v1: Changed yield to yield undefined

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

it looks enough for this case, thanks.
Actually, if you would wish to fix other cases in Places, I could find other 2 instances of the problem:
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/tests/browser/browser_colorAnalyzer.js#24
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/tests/unit/test_isURIVisited.js#90
I don't care about further reviewing those test changes, so feel free to just do those and push.

You should also fix the commit message to actually include the bug number and the review flag
Comment 4 Marco Bonardo [::mak] 2013-06-27 12:17:15 PDT
oh and sorry if I missed the question in comment 1
I'd like to suggest you to use the needinfo? flag that is at the end of the bug report to ask direct questions to developers, that is easier for us to track.

you can find most of the information on how to contribute at https://developer.mozilla.org/en-US/docs/Introduction but feel free to ask if you have any doubt. you can also reach us on irc.mozilla.org in channels #developers and #fx-team for any questions
Comment 5 Sushant Hiray [:sushant] 2013-06-27 12:23:13 PDT
(In reply to Marco Bonardo [:mak] from comment #3)
> Comment on attachment 768499 [details] [diff] [review]
> v1: Changed yield to yield undefined
> 
> Review of attachment 768499 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> it looks enough for this case, thanks.
> Actually, if you would wish to fix other cases in Places, I could find other
> 2 instances of the problem:
> http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/
> tests/browser/browser_colorAnalyzer.js#24
> http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/
> tests/unit/test_isURIVisited.js#90
> I don't care about further reviewing those test changes, so feel free to
> just do those and push.
> 
> You should also fix the commit message to actually include the bug number
> and the review flag

I'm not sure if I can push directly. I don't have commit access as of now. So should I submit a patch for those 2 files in this bug only?
Comment 6 Marco Bonardo [::mak] 2013-06-27 12:28:36 PDT
yes, you can add those other 2 changes to the existing patch, and attach an updated version with all of them here.

For check-in you can refer to https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F
and once the patch is in the right shape, just add the checkin-needed keyword to the above keywords field, and some sheriff will take care of pushing your patch to the repository.
Comment 7 Sushant Hiray [:sushant] 2013-06-27 12:44:02 PDT
Created attachment 768518 [details] [diff] [review]
Bug:887320 Changed deprecated yield to yeild undefined
Comment 8 Sushant Hiray [:sushant] 2013-07-01 07:18:29 PDT
Hey Marco,

I've added those changes some time back. I had proposed them to you for a review.
Could you have a look and then somebody can checkin for me!

Regards,
Sushant
Comment 9 :Gijs Kruitbosch 2013-07-02 07:48:15 PDT
(In reply to Sushant Hiray [:sushant] from comment #7)
> Created attachment 768518 [details] [diff] [review]
> Bug:887320 Changed deprecated yield to yeild undefined

Hi Sushant,

In comment #4, Marco already said, it's fine to just change these two lines without further review. However, you will need to change the summary of the patch to include the bug number and the reviewer. So, please make sure that the patch header has this information (Bug 887320, r=mak) in addition to the description. Then attach the new patch, set the review flag to '+', and add 'checkin-needed' to the keywords field, please. :-)

Thank you!
Comment 10 Sushant Hiray [:sushant] 2013-07-02 10:53:53 PDT
Created attachment 770287 [details] [diff] [review]
Bug:887320 r=mak {Changed yield to yield undefined}
Comment 11 :Gijs Kruitbosch 2013-07-02 10:57:22 PDT
(In reply to Sushant Hiray [:sushant] from comment #10)
> Created attachment 770287 [details] [diff] [review]
> Bug:887320 r=mak {Changed yield to yield undefined}

OK, I was unclear, I'm afraid. If you open the attachment, you will see these lines at the top:

# HG changeset patch
# Parent 110d0591ce68008ac68ca6da0c8b7c06e169130f
# User Sushant Hiray <hiraysushant@gmail.com>
modifying deprecated yield without value to yield undefined


The bottom line should be something along the lines of:

Bug 887320 - yield without a value is deprecated, use yield undefined, r=mak


If you're using mercurial queues, you can use "hg qref -e" to do that. For plain mercurial commits, I'm not sure - you'd have to remove the commit (hg strip can do that) and recommit with the correct message (hg commit -m "..."), I expect.

Finally, don't set the checkin flag on the attachment. Once you've added the patch, back on the bug page, there's a "Keywords" field, near the top. Add 'checkin-needed' in there.
Comment 12 Sushant Hiray [:sushant] 2013-07-02 11:01:37 PDT
Created attachment 770294 [details] [diff] [review]
Bug:887320 r=mak {Changed yield to yield undefined}
Comment 13 Sushant Hiray [:sushant] 2013-07-02 11:03:32 PDT
(In reply to :Gijs Kruitbosch from comment #11)
> (In reply to Sushant Hiray [:sushant] from comment #10)
> > Created attachment 770287 [details] [diff] [review]
> > Bug:887320 r=mak {Changed yield to yield undefined}
> 
> OK, I was unclear, I'm afraid. If you open the attachment, you will see
> these lines at the top:
> 
> # HG changeset patch
> # Parent 110d0591ce68008ac68ca6da0c8b7c06e169130f
> # User Sushant Hiray <hiraysushant@gmail.com>
> modifying deprecated yield without value to yield undefined
> 
> 
> The bottom line should be something along the lines of:
> 
> Bug 887320 - yield without a value is deprecated, use yield undefined, r=mak
> 
> 
> If you're using mercurial queues, you can use "hg qref -e" to do that. For
> plain mercurial commits, I'm not sure - you'd have to remove the commit (hg
> strip can do that) and recommit with the correct message (hg commit -m
> "..."), I expect.
> 
> Finally, don't set the checkin flag on the attachment. Once you've added the
> patch, back on the bug page, there's a "Keywords" field, near the top. Add
> 'checkin-needed' in there.

Ooh I see,
Thanks a lot for your help!
I was unaware of the checkin-needed in "Keywords" field!
Thanks for stopping by and helping!
I hope this patch fulfills all the guidelines.
Comment 14 Marco Bonardo [::mak] 2013-07-02 11:42:39 PDT
Sorry for late answer, I was traveling and got hijacked by higher priority stuff.
But I see Gijs has done a great job at replacing me! Thanks you both, and congratulations on your first patch.
Comment 15 Sushant Hiray [:sushant] 2013-07-02 11:47:47 PDT
Thanks a lot :-)
Comment 16 :Gijs Kruitbosch 2013-07-02 12:28:40 PDT
*** Bug 889527 has been marked as a duplicate of this bug. ***
Comment 17 Ryan VanderMeulen [:RyanVM] 2013-07-02 12:52:49 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/c361c2af8910
Comment 18 Ryan VanderMeulen [:RyanVM] 2013-07-03 11:36:01 PDT
https://hg.mozilla.org/mozilla-central/rev/c361c2af8910

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