Last Comment Bug 750910 - add warning about removal of java DOM objects to Aurora
: add warning about removal of java DOM objects to Aurora
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: ---
Assigned To: Josh Aas
:
:
Mentors:
Depends on:
Blocks: 748343
  Show dependency treegraph
 
Reported: 2012-05-01 15:02 PDT by Josh Aas
Modified: 2012-05-09 23:14 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix v1.1 for Aurora (1.42 KB, patch)
2012-05-01 15:02 PDT, Josh Aas
bugs: review-
Details | Diff | Splinter Review
fix v1.2 for Aurora (2.64 KB, patch)
2012-05-02 08:17 PDT, Josh Aas
bugs: review+
l10n: review-
Details | Diff | Splinter Review
fix v1.3 (2.64 KB, patch)
2012-05-07 07:37 PDT, Josh Aas
l10n: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Josh Aas 2012-05-01 15:02:51 PDT
Created attachment 620080 [details] [diff] [review]
fix v1.1 for Aurora
Comment 1 Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2012-05-01 15:13:25 PDT
Comment on attachment 620080 [details] [diff] [review]
fix v1.1 for Aurora

>diff --git a/content/base/public/nsDeprecatedOperationList.h b/content/base/public/nsDeprecatedOperationList.h
>--- a/content/base/public/nsDeprecatedOperationList.h
>+++ b/content/base/public/nsDeprecatedOperationList.h
>@@ -72,8 +72,9 @@ DEPRECATED_OPERATION(IsEqualNode)
> DEPRECATED_OPERATION(TextContent)
> DEPRECATED_OPERATION(EnablePrivilege)
> DEPRECATED_OPERATION(Position)
> DEPRECATED_OPERATION(TotalSize)
> DEPRECATED_OPERATION(InputEncoding)
> DEPRECATED_OPERATION(MozBeforePaint)
> DEPRECATED_OPERATION(MozBlobBuilder)
> DEPRECATED_OPERATION(DOMExceptionCode)
>+DEPRECATED_OPERATION(JavaPackages)

You need to add something to dom.properties too



>     if (id == sJava_id || id == sPackages_id) {
>+      nsCOMPtr<nsIContent> content(do_QueryWrappedNative(wrapper, obj));
>+      if (content) {
>+        content->OwnerDoc()->WarnOnceAbout(nsIDocument::eJavaPackages);
>+      }
Er, does this code ever get executed.
Shouldn't you do something like
if (win->GetExtantDocument()) {
  nsCOMPtr<nsIDocument> d = do_QueryInterface(win->GetExtantDocument());
  d->WarnOnceAbout(nsIDocument::eJavaPackages);
}


>+
>       static bool isResolvingJavaProperties;
> 
>       if (!isResolvingJavaProperties) {
>         isResolvingJavaProperties = true;
> 
>         // Tell the window to initialize the Java properties. The
>         // window needs to do this as we need to do this only once,
>         // and detecting that reliably from here is hard.
Comment 2 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-05-02 00:02:50 PDT
(In reply to Olli Pettay [:smaug] from comment #1)
> You need to add something to dom.properties too

That's going to make landing this on Aurora problematic, isn't it?
Comment 3 Josh Aas 2012-05-02 08:17:45 PDT
Created attachment 620320 [details] [diff] [review]
fix v1.2 for Aurora

Gavin may be right but here's an updated patch anyway. It works.
Comment 4 Masatoshi Kimura [:emk] 2012-05-02 21:41:52 PDT
> +JavaPackagesWarning=Use of the 'java' and 'packages' DOM objects to access Java functionality is deprecated. Support will be removed soon.
The first letter of 'Packages' is upper cased.
Comment 5 Josh Aas 2012-05-03 11:11:42 PDT
Comment on attachment 620320 [details] [diff] [review]
fix v1.2 for Aurora

Requesting aurora approval, string change is the biggest issue here.

[Approval Request Comment]
Regression caused by (bug #): 748343
User impact if declined: less warning about a change in the next release
Testing completed (on m-c, etc.): none
Risk to taking this patch (and alternatives if risky): not risky
String changes made by this patch: yes
Comment 6 Alex Keybl [:akeybl] 2012-05-06 19:54:48 PDT
(In reply to Josh Aas (Mozilla Corporation) from comment #5)
> String changes made by this patch: yes

Given this, CC'ing Axel to make sure he's OK with this change. I'm assuming we'd be OK with localizations falling back to en-US, but I still don't want this to be unexpected.
Comment 7 Axel Hecht [:Pike] 2012-05-06 23:05:27 PDT
Comment on attachment 620320 [details] [diff] [review]
fix v1.2 for Aurora

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

Adding an r- here to make sure we're actually fixing the string to say "Packages".

That said, it's an error console string, and should be OK. Please post to .l10n about this, too.

Is the intention to land this on central, too, or is this just in for this one release and going to be dropped again with 15?
Comment 8 Josh Aas 2012-05-07 06:37:50 PDT
The intent is just to add this for Firefox 14. Firefox 15 actually contains the removal, no warning.
Comment 9 Josh Aas 2012-05-07 07:37:58 PDT
Created attachment 621594 [details] [diff] [review]
fix v1.3
Comment 10 Josh Aas 2012-05-09 23:14:13 PDT
pushed to mozilla-aurora

http://hg.mozilla.org/releases/mozilla-aurora/rev/0bd74da75dc9

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