Closed Bug 803188 Opened 8 years ago Closed 7 years ago

Provide a way to convert from a file path to a file:// URI (and vice-versa)

Categories

(Toolkit :: OS.File, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: msucan, Assigned: dustin)

References

Details

(Keywords: dev-doc-needed, Whiteboard: [Async][mentor=Yoric][lang=js])

Attachments

(1 file, 8 obsolete files)

Today I worked with the OS.File API and I found that I have to use NetUtil/FileUtils to go from a local file path to a file:// URI path.

Given:

  path = OS.Path.join(OS.Constants.Path.tmpDir, "foo.html");

I have to do:

  file = new FileUtils.File(path);
  uri = NetUtil.newURI(file).spec;


On a similar note: I'd like a way to convert chrome:// URIs to file:// (and vice-versa). I needed this in other pieces of code I did some time ago (before OS.File).
We could add |OS.Path.toFileURI|.
I don't think that we can do chrome:// [yet], but let us concentrate on file://

The algorithm used in NetUtil is defined at:
- http://dxr.mozilla.org/mozilla-central/netwerk/base/src/nsURLHelperUnix.cpp.html#l14 (for Unix)
- http://dxr.mozilla.org/mozilla-central/netwerk/base/src/nsURLHelperWin.cpp.html#l14 (for Windows)

It should be quite easy to port it to JavaScript, as part of ospath_win.jsm/ospath_unix.jsm.
Whiteboard: [mentor=Yoric][lang=js]
Can I get assigned this bug?
(In reply to WangJun from comment #3)
> Can I get assigned this bug?

Certainly.
If you have any question, feel free to contact me.
Assignee: nobody → w1990ww1990w
Why is it bad to use FileUtils/NetUtil in this case? Duplicating chrome:// path resolution is ~impossible since it's implemented with a healthy dose of extensible XPCOM.
(In reply to Taras Glek (:taras) from comment #5)
> Why is it bad to use FileUtils/NetUtil in this case?

The main reason is that FileUtils/NetUtil can only be used from the main thread.

Now, there are a few secondary reasons, in particular the fact that having to use two additional modules just to convert to/from a file:// path for use with OS.File, makes for a poor developer experience.

> Duplicating chrome://
> path resolution is ~impossible since it's implemented with a healthy dose of
> extensible XPCOM.

I have no intention to duplicate chrome:// resolution, which is generally unfeasible as you mention, just file://, which should be quite simple.
WangJun, after your message on IRC, I feel that I need to clarify one thing: the objective of this bug is *not* to wrap the C++ function into something that can be called from JS.

What we want here is to implement the algorithm in pure JavaScript. I can be wrong, but I believe that this can be done in ~10 lines for each conversion for Unix and probably about the same number for Windows. I provided the links to C++ source code so that you could check how the algorithm looks like in C++.

Basically, converting file path to file:// URI:
- use OS.Path.split to divide the path into its components;
- encode each component with built-in JavaScript function |encodeURIComponent|;
- concatenate back everything.

Converting file:// URI to file path:
- remove "file://";
- use String split to divide the uri into its components;
- decode each component with built-in JavaScript function |decodeURIComponent|;
- concatenate back everything.
WangJun, are you still interested in working on that bug?
Flags: needinfo?(w1990ww1990w)
In the absence of reply, resetting the bug. If anyone is interested, please take it :)
Assignee: w1990ww1990w → nobody
Flags: needinfo?(w1990ww1990w)
Hi, this is my first time contributing to OSS. Would like to get assigned to this bug.

Thank you!
With pleasure, kkos. Have fun!
If you have any question, do not hesitate to come to IRC (server irc.mozilla.org, channel #introduction or, equivalently, https://irc.lc/mozilla/introduction/kkos ).
I'm not sure how to write tests. Any advice?
Attachment #709014 - Flags: review?(dteller)
Assignee: nobody → kkos
I'm not sure that this duplication is a good idea, but I believe that this patch should be reviewed by a necko peer in any case, and we should note in both locations that any changes to the code should be duplicated in the other path.
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #14)
> I'm not sure that this duplication is a good idea, but I believe that this
> patch should be reviewed by a necko peer in any case, and we should note in
> both locations that any changes to the code should be duplicated in the
> other path.

Not sure. It's not a feature duplication (nsIFile doesn't work on worker threads), it's not part of Necko, and it doesn't deal with chrome:// uris.
Oh, and Necko makes the mistake of believing Microsoft's documentation when it states that paths cannot be longer than MAX_PATH. That's probably something we'll want to avoid with OS.Path/OS.File. But that's another story.
Comment on attachment 709014 [details] [diff] [review]
Added functions to both windows and unix that will convert local filre path to URI.

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

This may works, but it definitely needs tests.

You can find more information about how to write simple tests here: https://developer.mozilla.org/en/Writing_xpcshell-based_unit_tests

I would check that nsIIOService and your code return the same file:// uri for a variety of paths.

Also, I revise my earlier reply to bsmedberg. Benjamin, if you believe that a Necko peer should look at the algorithm, could you suggest someone? I don't think we need a full review from a peer, but having some feedback would be interesting.
Attachment #709014 - Flags: review?(dteller) → feedback+
Any progress, Keng Kiat?
Flags: needinfo?(kkos)
Hi David,

Based on the discussion in IRC previously, I'm required to implement the method to convert the file path the the uri format, and vice-versa. While that is done, I have some trouble passing some tests

For example ~/Documents does not translate to file:///Users/username/Documents/. Instead, I got file:///~/Documents/. So I'm trying to find a way to get that parsed.
Flags: needinfo?(kkos)
Ah, good point. I didn't realize that nsIIOService handled "~". It probably shouldn't, so this is ok if you do not handle tests that use symbol "~".
The other problem is in test case '/test\ test/', my unit test would throw an exception. Currently I'm using FileUtils.File() to compare against my function. I suspect that FileUtils.File() cannot handle the space character. Any suggestions?
That's surprising. Can you double-check that |FileUtils.File()| fails with files that contain a space in the name?

Btw, don't hesitate to tick "Need more information from [me]", this will ensure that I don't forget replying to your questions.
Ok, I've found the problem. Apparently FileUtils.File() will fail when i supply a path that doesn't start with a '/'.

However, I'm now facing another problem when testing the conversion from file URI back to file path. Currently, I'm using iOService.newURI() to create a URI object, and comparing my code with URI.spec. However, when testing with this test case: "file:///test%20test/", iOService.newURI() does not decode the URI into its special characters (In this case a space character). I believe that the decoding functionality must be included in this conversion process since file names can contain spaces. Is there a better way to test this?
Flags: needinfo?(dteller)
iOService.newFileURI()?
Ah, it's a bit tricky indeed.
So, once you have your nsIURI, you will need to convert it to a nsIFileURL, as follows:
   let myFileURL = myURI.QueryInterface(nsIFileURL)

Interface nsIFileURL should contain everything you need:
   https://developer.mozilla.org/en-US/docs/XPCOM_Interface_Reference/nsIFileURL
Flags: needinfo?(dteller)
(FWIW, You don't need the extra variable. you could just do myURI.QueryInterface(nsIFileURL); myURI.QueryInterface(nsIFileURL); myURI.WhatEeverFileURIProvices();)
Any news, Keng Kiat?
Flags: needinfo?(kkos)
I've done up both methods, and tested on a Unix machine, now I have to test it on a Windows machine...
Flags: needinfo?(kkos)
Keng Kiat, if you are satisfied with your test suite and if you do not have access to a Windows machine, I can send your patch to one of our Windows unit testing servers for automated testing.
Flags: needinfo?(kkos)
I have tested this in Unix (Mac). However I am unable to perform test on my Windows VM. I have written tests for both environments. Tests are located in the other patch.
Attachment #709014 - Attachment is obsolete: true
Attachment #746436 - Flags: review?(dteller)
Flags: needinfo?(kkos)
Attached patch Test cases for the fixes (obsolete) — Splinter Review
Attachment #746437 - Flags: review?(dteller)
You'll find the results of testing for Linux, MacOS and Windows here: https://tbpl.mozilla.org/?tree=Try&rev=a466bf5be4c5
Comment on attachment 746436 [details] [diff] [review]
Bug Fixes, tests located in other patch

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

Good start.
Oh, by the way, you should change the commit message of your patch to something along the lines of
"Bug 803188 - Conversion between file paths and file:/// uris;r=yoric"

::: toolkit/components/osfile/ospath_unix_back.jsm
@@ +135,5 @@
> +     /**
> +      * Return the file:// URI file path of the given local file path.
> +      */
> +     filePathToFileURI: function filePathToFileURI(path) {
> +       

Nit: Please avoid these whitespaces.
(you can find them easily by following the "review" link on bugzilla)

@@ +137,5 @@
> +      */
> +     filePathToFileURI: function filePathToFileURI(path) {
> +       
> +       // Remove trailing slashes
> +       if(path.substr(-1) === '/' && path.length > 1) {

Nit: Please add a whitespace after |if|.

@@ +139,5 @@
> +       
> +       // Remove trailing slashes
> +       if(path.substr(-1) === '/' && path.length > 1) {
> +         path = path.substr(0, path.length - 1);
> +        }

That won't work if you have several trailing slashes.
I seem to remember that |normalize| will do what you want, though.

@@ +141,5 @@
> +       if(path.substr(-1) === '/' && path.length > 1) {
> +         path = path.substr(0, path.length - 1);
> +        }
> +       
> +       var prefix = "file://";

Please use |let| instead of |var| in this file.

@@ +150,5 @@
> +       uri = prefix + uri.replace(/;/g, "%3b")
> +                         .replace(/\?/g, "%3F");
> +
> +       return uri;
> +     },

Please also document this function.

@@ +153,5 @@
> +       return uri;
> +     },
> +     fileURIToFilePath: function fileURIToFilePath(uri) {
> +       
> +       if(uri.indexOf("file://localhost/") === 0) {

Interesting. I had no idea that this should work.

@@ +162,5 @@
> +       
> +       // strip trailing slashes
> +       if(uri.charAt(uri.length - 1) === '/' && uri.length > 1) {
> +         uri = uri.substr(0, uri.length - 1);
> +       }

If possible, I would let |normalize| handle these trailing spaces.

::: toolkit/components/osfile/ospath_win_back.jsm
@@ +246,5 @@
> +       
> +       // Remove trailing slashes
> +       if(path.substr(-1) === '\\' && path.length > 3) {
> +         path = path.substr(0, path.length - 1);
> +       }

Generally, same remarks as Unix version.

::: toolkit/components/osfile/tests/xpcshell/xpcshell.ini
@@ +9,5 @@
> +[test_file_URL_conversion_unix.js]
> +skip-if = os == 'win'
> +
> +[test_file_URL_conversion_win.js]
> +run-if = os == 'win'
\ No newline at end of file

Please move this to the other patch.
Also, please add a newline at the end of the file.
Attachment #746436 - Flags: review?(dteller) → feedback+
Comment on attachment 746437 [details] [diff] [review]
Test cases for the fixes

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

Looks good, thanks.

::: toolkit/components/osfile/tests/xpcshell/test_file_URL_conversion_unix.js
@@ +3,5 @@
> +  // this is just an example, so we assert that true == true
> +  Components.utils.import("resource://gre/modules/Services.jsm");
> +  Components.utils.import("resource://gre/modules/osfile.jsm");
> +  Components.utils.import("resource://gre/modules/FileUtils.jsm");
> +  

As in the other patch, please avoid whitespaces, please use |let|.

@@ +4,5 @@
> +  Components.utils.import("resource://gre/modules/Services.jsm");
> +  Components.utils.import("resource://gre/modules/osfile.jsm");
> +  Components.utils.import("resource://gre/modules/FileUtils.jsm");
> +  
> +  var iOService = Services.io;

You don't really need that shortcut.

@@ +18,5 @@
> +    '/test\ test'
> +    );
> +  
> +  // Empty array to be used later for fileURIToFilePath tests
> +  uriStringList = new Array();

You should use |[]| instead of |new Array()|.

@@ +21,5 @@
> +  // Empty array to be used later for fileURIToFilePath tests
> +  uriStringList = new Array();
> +  
> +  // Testing of filePathToURI
> +  for(var key in pathStringList) {

Please add a whitespace after |for|.

::: toolkit/components/osfile/tests/xpcshell/test_file_URL_conversion_win.js
@@ +14,5 @@
> +    'C:\\test\\',
> +    'C:\\test\\test\\test',
> +    'C:\\test;+%',
> +    'C:\\test?action=index\\',
> +    'C:\\test\ test'

You should also check with UNC-formatted paths, e.g.
\\C:\a\b\c
\\Server\a\b\c
Attachment #746437 - Flags: review?(dteller) → feedback+
I haven't seen any updates to this bug in some time. What's your status? Do you need assistance?
Flags: needinfo?(kkos)
Attached patch bug803188.patch (obsolete) — Splinter Review
Updated changes. There may be an issue with the window's version of normalise. Please see the comments in the code.
Attachment #746436 - Attachment is obsolete: true
Attachment #788671 - Flags: review?
Flags: needinfo?(kkos)
Attached patch bug803188test.patch (obsolete) — Splinter Review
Attachment #746437 - Attachment is obsolete: true
Attachment #788672 - Flags: review?(dteller)
Comment on attachment 788671 [details] [diff] [review]
bug803188.patch

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

Looks good.
r=me with the following changes, if it passes tests

By the way, the files you are patching have changed a little (in particular, they have changed name), so you will need to fix conflicts next time you pull.

::: toolkit/components/osfile/modules/ospath_unix_back.jsm
@@ +151,5 @@
> +     },
> +     /**
> +      * Returns the file:// URI file path of the given local file path.
> +      */
> +     filePathToFileURI: function filePathToFileURI(path) {

Given that the module name is already |Path|, we can probably simplify the name of these functions to |fromFileURI| and |toFileURI|.

@@ +170,5 @@
> +       if (uri.indexOf("file://localhost/") === 0) {
> +         uri = uri.substr(16, uri.length);
> +       } else if (uri.indexOf("file:///") === 0) {
> +         uri = uri.substr(7, uri.length);
> +       }

Rather than using magical constants 7 and 16, I suggest storing "file://localhost/" and "file:///" as constant strings and using their length.

::: toolkit/components/osfile/modules/ospath_win_back.jsm
@@ +300,5 @@
> +       // encodeURI doesn't escape semicolons. So we need to manually
> +       // escape them.
> +       uri = prefix + uri.replace(/;/g, "%3b")
> +                         .replace(/\?/g, "%3F");
> +

Could you add a comment explaining this final "/"? I assume this is for transforming "file:///C:" into "file:///C:/", isn't it?

@@ +324,5 @@
> +                                 .replace(/\//g, "\\"));
> +
> +       // this.normalize() does not remove the trailing slash if the path
> +       // component is a drive letter. eg. 'C:\'' will not get normalized.
> +       if (path.substr(path.length - 2, path.length) === ":\\") {

Simply |if (path.endsWith(":\\") {|
Attachment #788671 - Flags: review? → review+
Comment on attachment 788672 [details] [diff] [review]
bug803188test.patch

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

Looks good, thanks.
Don't forget to rename your patch according to our conventions. The commit message should look like:
"Bug 803188 - Testing conversion between paths and file:// URIs;r=yoric"
Attachment #788672 - Flags: review?(dteller) → review+
How are things going Keng Kiat? I seem to remember that your work is almost complete, so it would be nice to land it.
Flags: needinfo?(kkos)
It seems that Keng Kiat is not workig on this anymore. However, the bug is almost ready, if anybody wants to unbitrot and land.
Assignee: kkos → nobody
Flags: needinfo?(kkos)
Whiteboard: [mentor=Yoric][lang=js] → [Async][mentor=Yoric][lang=js]
I have these patches un-bitrotten and squashed into a single patch.  I'll work to apply the review comments then submit for one more round of r?.  I'm a little confused about the renaming suggestions in comment 39, as the patch there already has comment "Bug 803188 - Tests for conversion between file paths and file:/// uris;r=yoric".  Are you suggesting that the patch filename in Bugzilla have that form?  Or is the distinction "Tests for" vs. "Testing"?

Is it OK to put the tests and functionality in the same patch?

Does this change need corresponding documentation in-tree, or does that go into MDN?
Attached patch bug803188.patch (obsolete) — Splinter Review
Here it is at any rate.  I did a little bit more refactoring to do the prefix checks in a loop, and noted the difference between UNIX and Windows as far as stripping the trailing slash of the prefix.
Assignee: nobody → dustin
Attachment #788671 - Attachment is obsolete: true
Attachment #788672 - Attachment is obsolete: true
Attachment #824799 - Flags: review?
Attachment #824799 - Flags: review? → review?(dteller)
Comment on attachment 824799 [details] [diff] [review]
bug803188.patch

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

r=me with the regexp change

::: toolkit/components/osfile/modules/ospath_unix.jsm
@@ +160,5 @@
> +
> +  // encodeURI doesn't escape semicolons. So we need to manually
> +  // escape them.
> +  uri = prefix + uri.replace(/;/g, "%3b")
> +                    .replace(/\?/g, "%3F");

We should probably do the replacement in one pass. Something along the lines of:
  uri.replace(/[;\?]/, match => (match == ":")?"%3b":"%3F)

@@ +182,5 @@
> +
> +  let path = decodeURI(this.normalize(uri));
> +  path = path.replace(/%3b/g, ";")
> +             .replace(/%3F/g, "?");
> +

As above.
Attachment #824799 - Flags: review?(dteller) → review+
(In reply to Dustin J. Mitchell [:dustin] (I read my bugmail; don't needinfo me) from comment #42)
> I have these patches un-bitrotten and squashed into a single patch.  I'll
> work to apply the review comments then submit for one more round of r?.  I'm
> a little confused about the renaming suggestions in comment 39, as the patch
> there already has comment "Bug 803188 - Tests for conversion between file
> paths and file:/// uris;r=yoric".  Are you suggesting that the patch
> filename in Bugzilla have that form?  Or is the distinction "Tests for" vs.
> "Testing"?

I don't remember why I put that comment.

> Is it OK to put the tests and functionality in the same patch?

For such a small patch, this is ok.

> Does this change need corresponding documentation in-tree, or does that go
> into MDN?

Well, some MDN doc will be needed
I'm curious why the URI escaping has the funny case it does (%3b vs. %3F).  In fromFileURI, we only match that case, which might cause a problem for opposite cases.  Should I add some tests for case-insensitivity and adjust the code to match, or are %3b and %3F canonical?
If my memory serves, %3b comes from the reference implementation: http://dxr.mozilla.org/mozilla-central/source/netwerk/base/src/nsURLHelperUnix.cpp#l45 – I'm not sure where %3F comes from, though. I believe that this maps to "?", so it would be interesting if you could find out the issue that prompted the addition of this case to the code by writing the appropriate tests.

Also, you're right, the choice of case is surprising. We probably want to handle both upper and lower case.

Now, I just realized that bug 920015 has landed, which means that we should be able to use the URL object (https://developer.mozilla.org/en-US/docs/Web/API/URL) for robust parsing of URLs. Could you check it out?
I've refactored the tests to check case sensitivity, and also switched to decodeURIComponent to convert %xx to the corresponding character, rather than using the ternary operator.  That's baking in try right now.

URL sounds great.  How do I get ahold of this URL object in ospath_unix.jsm?
I believe that |new URL(string)| should work.
But you need |Cu.importGlobalProperties(["URL"]);|. See bug 920015.
Thanks - I didn't know if that was OK in production code (as opposed to in tests).
(In reply to Masatoshi Kimura [:emk] from comment #50)
> But you need |Cu.importGlobalProperties(["URL"]);|. See bug 920015.

It looks like this is actually false - this seems to cause weird failures in mochitests on try:

10:09:18     INFO -  JavaScript error: , line 0: uncaught exception: 2147942487
https://tbpl.mozilla.org/php/getParsedLog.php?id=30281071&tree=Try
I stand corrected - that line is required iff `Components` is defined; when it is not defined, URL is already "automatically" available.
I'm sorry this hasn't seen any action.  I tripped and fell down the rabbit-hole of trying to get Firefox to build on Windows.  It builds, but tests don't run.
Attached patch bug803188-r2.patch (obsolete) — Splinter Review
I finally got the windows dev environment up and running, and a few iterations of the test suite later, I'm confident that the OSFile interfaces correctly mirror those in Services.io, including all of the relevant funny characters, case differences in uri encoding, and so on.  This also uses URL, as suggested.

Sorry for the delay.  Windows is annoying.

https://tbpl.mozilla.org/?tree=Try&rev=576e2e885059
Attachment #824799 - Attachment is obsolete: true
Attachment #8347246 - Flags: review?(dteller)
Comment on attachment 8347246 [details] [diff] [review]
bug803188-r2.patch

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

I believe it's good, I just have a few nits & questions before r+.

::: toolkit/components/osfile/modules/ospath_unix.jsm
@@ +35,5 @@
>    "join",
>    "normalize",
> +  "split",
> +  "toFileURI",
> +  "fromFileURI"

Nit: can you add a comma after "fromfileURI"?
Also, please remove the unnecessary newline.

@@ +169,5 @@
> +};
> +exports.toFileURI = toFileURI;
> +
> +/**
> + * Returns the local file path from a given file URI.

Could you throw if the uri scheme is not "file:"?

::: toolkit/components/osfile/modules/ospath_win.jsm
@@ +46,5 @@
>    "split",
>    "winGetDrive",
> +  "winIsAbsolute",
> +  "toFileURI",
> +  "fromFileURI"

Here, too, could you add a comma?

@@ +316,5 @@
> +
> +/**
> + * Returns the local file path from a given file URI.
> + */
> +let fileURIPrefixes = ["file:///", "file://localhost/"];

I believe that you don't need that array anymore.

@@ +317,5 @@
> +/**
> + * Returns the local file path from a given file URI.
> + */
> +let fileURIPrefixes = ["file:///", "file://localhost/"];
> +let fromFileURI = function fromFileURI(uri) {

Could you throw if the uri scheme is not "file:"?

@@ +325,5 @@
> +
> +  let path = decodeURI(uri);
> +  // decode a few characters where URL's parsing is overzealous
> +  path = path.replace(/%(3b|3f|23)/gi,
> +        match => decodeURIComponent(match));

Why don't we need this on Unix?

::: toolkit/components/osfile/tests/xpcshell/test_file_URL_conversion.js
@@ +1,1 @@
> +function run_test() {

Nit: Please add the public domain stuff at the start of the test.
Attachment #8347246 - Flags: review?(dteller) → feedback+
(In reply to David Rajchenbach Teller [:Yoric] (away 20 Dec to Jan 4th - please use "needinfo?") from comment #56)
> @@ +325,5 @@
> > +
> > +  let path = decodeURI(uri);
> > +  // decode a few characters where URL's parsing is overzealous
> > +  path = path.replace(/%(3b|3f|23)/gi,
> > +        match => decodeURIComponent(match));
> 
> Why don't we need this on Unix?

That's a good question, but this code is emulating the Services.io.newURI/newFileURI functionality, with all its warts and oddities (e.g., differing cases of URI encodings), and according to the tests it's doing so correctly.
Attached patch bug803188-r3.patch (obsolete) — Splinter Review
this should address all of the other comments
Attachment #8347246 - Attachment is obsolete: true
Attachment #8348862 - Flags: review?(dteller)
Comment on attachment 8348862 [details] [diff] [review]
bug803188-r3.patch

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

Looks good to me, thanks.
Don't forget to change the commit message.
Attachment #8348862 - Flags: review?(dteller) → review+
(same patch, just different commit message; r+ carried forward)
Attachment #8348862 - Attachment is obsolete: true
Attachment #8350100 - Flags: review+
https://hg.mozilla.org/integration/fx-team/rev/58698f94b762
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [Async][mentor=Yoric][lang=js] → [Async][mentor=Yoric][lang=js][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/58698f94b762
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [Async][mentor=Yoric][lang=js][fixed-in-fx-team] → [Async][mentor=Yoric][lang=js]
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.