Last Comment Bug 661876 - Kill obsolete nsIDOMFile properties/methods
: Kill obsolete nsIDOMFile properties/methods
Status: VERIFIED FIXED
[warning landed on aurora]
: dev-doc-complete, relnote
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla7
Assigned To: Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
:
Mentors:
Depends on: 662403 662838 664780 669438 670086 682679
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-03 10:58 PDT by Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
Modified: 2013-12-27 14:27 PST (History)
7 users (show)
khuey: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+


Attachments
Patch (13.76 KB, patch)
2011-06-03 10:58 PDT, Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
jonas: review+
Details | Diff | Splinter Review
Warn on aurora (6.99 KB, patch)
2011-06-03 15:15 PDT, Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
jonas: review-
Details | Diff | Splinter Review
Now with hardcoded strings (5.75 KB, patch)
2011-06-03 16:16 PDT, Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
jonas: review+
jst: approval‑mozilla‑aurora+
Details | Diff | Splinter Review
Test fixes (2.64 KB, patch)
2011-06-05 07:37 PDT, Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
no flags Details | Diff | Splinter Review
test case (741 bytes, text/html)
2011-08-30 01:05 PDT, Ioana (away)
no flags Details
test case used to verify the bug/fix (997 bytes, text/html)
2011-09-01 04:55 PDT, Ioana (away)
no flags Details

Description Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-06-03 10:58:42 PDT
Created attachment 537181 [details] [diff] [review]
Patch
Comment 1 Jonas Sicking (:sicking) No longer reading bugmail consistently 2011-06-03 11:16:40 PDT
Comment on attachment 537181 [details] [diff] [review]
Patch

Please also add a patch here which adds warnings to all these methods which we could check in to aurora.

I suspect none of these methods are used in inner loops or anything like that, so no need to add complexity by making sure that we only warn once per document or some such. Just add a warning to the console every time they are called.
Comment 2 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-06-03 15:15:12 PDT
Created attachment 537246 [details] [diff] [review]
Warn on aurora
Comment 3 Jonas Sicking (:sicking) No longer reading bugmail consistently 2011-06-03 15:31:20 PDT
Comment on attachment 537246 [details] [diff] [review]
Warn on aurora

Don't add strings to the dom.properties file as it's too late to take string changes on aurora. Even just adding it to the file, even if we don't expect people to translate it, will set off all sorts of warnings in various systems which we don't want.

Also, the properties are to developers called "fileName", "getAsDataURL" etc, no uppercase first character.
Comment 4 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-06-03 15:32:26 PDT
What do you want me to do then, hardcode it into the binary?
Comment 5 Jonas Sicking (:sicking) No longer reading bugmail consistently 2011-06-03 15:43:53 PDT
Yes
Comment 6 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-06-03 16:16:55 PDT
Created attachment 537266 [details] [diff] [review]
Now with hardcoded strings

Comments addressed.

Did I mention that nsPrintfCString is a horrible thing?
Comment 7 Jonas Sicking (:sicking) No longer reading bugmail consistently 2011-06-03 16:30:47 PDT
Comment on attachment 537266 [details] [diff] [review]
Now with hardcoded strings

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

I assume you tested that this works: r=me

::: content/base/src/nsDOMFile.cpp
@@ +706,5 @@
> +  // This is hardcoded here in English since we're past string freeze.
> +  
> +  nsPrintfCString warningText
> +  (500,
> +   "Use of File.%s is deprecated. To upgrade your code, use standard properties or use the DOM FileReader object. For more help https://developer.mozilla.org/en/DOM/FileReader",

"For more help, see https://..."
Comment 8 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-06-03 16:32:30 PDT
Yes, I tested this for all of the properties from the error console.
Comment 9 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-06-03 16:33:10 PDT
Comment on attachment 537266 [details] [diff] [review]
Now with hardcoded strings

We would like to take this patch on Aurora to warn web developers that these deprecated APIs are going away (in Firefox 7).
Comment 10 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-06-05 07:37:14 PDT
Created attachment 537455 [details] [diff] [review]
Test fixes
Comment 11 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-06-05 12:12:30 PDT
http://hg.mozilla.org/mozilla-central/rev/64dd0ffff3f2
http://hg.mozilla.org/mozilla-central/rev/1dec937dfbb6
Comment 12 Jonas Sicking (:sicking) No longer reading bugmail consistently 2011-06-05 22:42:00 PDT
Comment on attachment 537455 [details] [diff] [review]
Test fixes

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

::: content/canvas/test/test_mozGetAsFile.html
@@ +15,5 @@
> +  reader.onload = 
> +    function(e) {
> +      is(e.target.result, canvas.toDataURL(type),
> + "<canvas>.mozGetAsFile().getAsDataURL() should equal <canvas>.toDataURL()");
> +      SimpleTest.finish();

This doesn't look correct. Won't you call finish multiple times? Is that allowed?
Comment 13 Boris Zbarsky [:bz] (still a bit busy) 2011-06-06 15:26:28 PDT
This broke some of our tools; see bug 662403.
Comment 14 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-06-08 11:02:39 PDT
We might want to emphasize this change to web devs.
Comment 15 Axel Hecht [:Pike] 2011-06-09 15:07:13 PDT
To recap: no string changes on central, and hard-coded English deprecation notices to the error console for aurora?

If so, sounds good.
Comment 16 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-06-09 15:11:26 PDT
(In reply to comment #15)
> To recap: no string changes on central, and hard-coded English deprecation
> notices to the error console for aurora?
> 
> If so, sounds good.

Correct.
Comment 17 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-06-17 10:16:03 PDT
http://hg.mozilla.org/releases/mozilla-aurora/rev/fed3cae237b1

Not marking fixed on Aurora or anything since these are just warnings.
Comment 18 Asa Dotzler [:asa] 2011-07-12 14:51:57 PDT
going to track this for potential fallout similar to what we saw at bug 670086
Comment 19 Ioana (away) 2011-08-29 06:15:18 PDT
Can anyone please help me with a test case, STR or some guidelines that I can use to verify this fix?

Thank you
Comment 20 Jonas Sicking (:sicking) No longer reading bugmail consistently 2011-08-29 10:35:30 PDT
If you get hold of a File object, using for example <input type=file> and getting .files[0], then the File object should *not* have any of the following properties or functions:

.fileName
.fileSize
.getAsText()
.getAsDataURL()
.getAsBinary()
Comment 21 Ioana (away) 2011-08-30 01:05:52 PDT
Created attachment 556771 [details]
test case

test case used to verify the bug/fix
Comment 22 Ioana (away) 2011-08-30 01:27:34 PDT
Verified fixed on:
Mozilla/5.0 (Windows NT 6.1; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (X11; Linux i686 on x86_64; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (Windows NT 5.1; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:7.0) Gecko/20100101 Firefox/7.0

Steps used to verify the fix:
 1. Open the test case from comment #21.
 2. Upload a file.
 3. Tap on the Display Properties button.
The Name and Size properties are displayed as "undefined". The .getAsText(), .getAsDataURL() and .getAsBinary() functions don't return anything.
 
(I verified the test case on FF6 and all the properties were displayed).
Comment 23 Ioana (away) 2011-09-01 04:55:59 PDT
Created attachment 557462 [details]
test case used to verify the bug/fix

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