The default bug view has changed. See this FAQ.

Fennec fails to build with javac 1.7 because of warnings

RESOLVED FIXED in Firefox 13

Status

()

Firefox for Android
General
P3
normal
RESOLVED FIXED
5 years ago
8 months ago

People

(Reporter: kats, Assigned: kats)

Tracking

unspecified
Firefox 13
All
Android
Points:
---

Firefox Tracking Flags

(firefox11 affected, firefox12 affected, firefox13 fixed, fennec+)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Created attachment 592827 [details]
warnings

Attached are the warnings output by the 1.7 compiler as reported by bgirard when he tried to build with javac 1.7. We should fix these as many (if not all) are legitimate warnings.

Also we should prevent regressing these by turning on any necessary -Xlint options in the 1.6 compile path.
Adding :lucasr since a bunch of these warnings are in his code and I'm not sure offhand what some of the List and Map types should be specific-ized as.

Updated

5 years ago
tracking-fennec: --- → +
Priority: -- → P3
Created attachment 597048 [details] [diff] [review]
Fix most compiler warnings
Attachment #597048 - Flags: review?(chrislord.net)
Created attachment 597049 [details] [diff] [review]
Work around javac 1.7 warning suppression bug
Attachment #597049 - Flags: review?(chrislord.net)
With respect, this is the wrong fix.

+                @SuppressWarnings("rawtypes")
                 Map map = (Map) selectedItem;

Add generic types instead. The only use of suppressing this kind of thing should be for library code that we don't control.
Depends on: 727121

Comment 5

5 years ago
Comment on attachment 597048 [details] [diff] [review]
Fix most compiler warnings

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

Looks good to me. The @SuppressWarnings stuff is unfortunate, but I assume it's necessary.
Attachment #597048 - Flags: review?(chrislord.net) → review+
Comment on attachment 597049 [details] [diff] [review]
Work around javac 1.7 warning suppression bug

Obsoleted by Bug 727121. Feel free to land the patch from there if you need a fix before I commit it.
Attachment #597049 - Attachment is obsolete: true
Attachment #597049 - Flags: review?(chrislord.net)

Comment 7

5 years ago
Comment on attachment 597049 [details] [diff] [review]
Work around javac 1.7 warning suppression bug

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

Looks good to me.

::: mobile/android/base/sync/setup/activities/SetupSyncActivity.java
@@ +301,5 @@
>      String syncKey   = mAccountManager.getUserData(account, Constants.OPTION_SYNCKEY);
>      String serverURL = mAccountManager.getUserData(account, Constants.OPTION_SERVER);
>  
>      JSONObject jAccount = new JSONObject();
> +    Map<String, String> jAccountMap = (Map<String, String>)jAccount;

Maybe add a comment as to why this is being done?
Attachment #597049 - Flags: review+
(In reply to Richard Newman [:rnewman] from comment #4)
> With respect, this is the wrong fix.
> 
> +                @SuppressWarnings("rawtypes")
>                  Map map = (Map) selectedItem;
> 

So I tracked down where this map was coming from and was able to cast it like so:

Map<String,Object> map = (Map<String,Object>)selectedItem

but then it just gave me an unchecked warning instead of a raw types warning, which I had to suppress anyway. So I ended up leaving it this way. The problem is mostly that this Map gets put into the Android view hierarchy and so when we get it back it's just a plain Object, and we need to do some sort of cast to make it usable.
https://hg.mozilla.org/integration/mozilla-inbound/rev/57c9ea722e73
status-firefox11: --- → affected
status-firefox12: --- → affected
status-firefox13: --- → fixed
https://hg.mozilla.org/mozilla-central/rev/57c9ea722e73
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 13
Comment on attachment 597048 [details] [diff] [review]
Fix most compiler warnings

[Approval Request Comment]
Regression caused by (bug #): none
User impact if declined: none
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): future patches may not apply cleanly. alternative is to rebase those future patches, which shouldn't be too hard.
String changes made by this patch: none
Attachment #597048 - Flags: approval-mozilla-beta?
Attachment #597048 - Flags: approval-mozilla-aurora?
Comment on attachment 597048 [details] [diff] [review]
Fix most compiler warnings

Cancelling uplift request based on latest status meeting, since this isn't really a critical bug or anything.
Attachment #597048 - Flags: approval-mozilla-beta?
Attachment #597048 - Flags: approval-mozilla-aurora?
You need to log in before you can comment on or make changes to this bug.