Use new async getCharsetForURI in /toolkit/components/places/tests/unit/test_bookmarks_json.js

RESOLVED FIXED in mozilla23

Status

()

Toolkit
Places
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: raymondlee, Assigned: Marcos Aruj)

Tracking

Trunk
mozilla23
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

4 years ago
Use new async PlacesUtils.getCharsetForURI in this test
/toolkit/components/places/tests/unit/test_bookmarks_json.js
(Reporter)

Updated

4 years ago
Assignee: nobody → marcos
(Reporter)

Comment 1

4 years ago
Have a look at this as a reference
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/tests/unit/test_bookmarks_html.js#374
(Assignee)

Comment 2

4 years ago
Created attachment 730275 [details] [diff] [review]
Use new getCharsetForURI.

Hi. Here's a patch to close this bug. Let me know if it's OK. Thanks.
Attachment #730275 - Flags: review?(mak77)
Comment on attachment 730275 [details] [diff] [review]
Use new getCharsetForURI.

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

::: toolkit/components/places/tests/unit/test_bookmarks_json.js
@@ +232,4 @@
>      }
> +  }).then(deferred.resolve);
> +
> +  return deferred.promise;

rather than defining a deferred and returning its promise, directly
return Task.spawn(...

so basically return the Task promise.
Please also do the same here:
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/tests/unit/test_bookmarks_html.js#406
Attachment #730275 - Flags: review?(mak77) → feedback+
(Reporter)

Updated

4 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 4

4 years ago
Created attachment 736336 [details] [diff] [review]
Use new getCharsetForURI, return Task.spawn directly.

Hi Marco,

I've changed the patch so it directly returns Task.spawn instead of the deferred's promise. Let me know if it's ok. 

Cheers,

Marcos.
Attachment #730275 - Attachment is obsolete: true
Attachment #736336 - Flags: review?
Attachment #736336 - Flags: review? → review?(mak77)
Comment on attachment 736336 [details] [diff] [review]
Use new getCharsetForURI, return Task.spawn directly.

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

it look ok!

::: toolkit/components/places/tests/unit/test_bookmarks_json.js
@@ +121,4 @@
>          break;
>        case "unfiled":
>          root =
> +            PlacesUtils.getFolderContents(PlacesUtils.unfiledBookmarksFolderId).root;

these 3 indentation changes could be avoided
Attachment #736336 - Flags: review?(mak77) → review+
(Reporter)

Comment 6

4 years ago
(In reply to Marco Bonardo [:mak] from comment #5)
> Comment on attachment 736336 [details] [diff] [review]
> Use new getCharsetForURI, return Task.spawn directly.
> 
> Review of attachment 736336 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> it look ok!
> 
> ::: toolkit/components/places/tests/unit/test_bookmarks_json.js
> @@ +121,4 @@
> >          break;
> >        case "unfiled":
> >          root =
> > +            PlacesUtils.getFolderContents(PlacesUtils.unfiledBookmarksFolderId).root;
> 
> these 3 indentation changes could be avoided

Pushed to try and waiting for results
https://tbpl.mozilla.org/?tree=Try&rev=fc7165d459f5

Please fix the indentation changes and mark this bug for check-in if it passes try tests.
(Reporter)

Comment 7

4 years ago
Created attachment 737351 [details] [diff] [review]
Patch for check-in
Attachment #736336 - Attachment is obsolete: true
(Reporter)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e93879f1adb
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/2e93879f1adb
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.