861994 - Sync pairing crashes fennec in 23.0a1 2013-04-15

Status: RESOLVED DUPLICATE of bug 861658

(Firefox for Android Graveyard :: General, defect)

Description

[Gene Wood [:gene]]

23.0a1 2013-04-15

In nightly when attempting to access the sync pairing options fennec crashes

To reproduce :

Settings...
Sync...
Now in the Accounts and sync settings page selecting an existing working sync account...
Pair a Device...

Expected results :
Some kind of a pair screen

Actual results :
White screen, then back to the settings screen, then to a white screen again, then "The application Nightly (process org.mozilla.fennec) has stopped unexpectedly. Please try again." Force Close

Comment 1

[Scoobidiver (away)]

Type about:crashes and provide the crash ID. If there's none, provide the logcat.

Comment 2

[Gene Wood [:gene]]

Checking about:crashes it only shows a crash from january so no crash report was submitted.

Is there someone in the SF office that could run the logcat for me? I'm not very uber and it looks like it has something to do with android debugging. I can bring anyone my phone.

Comment 3

[Scoobidiver (away)]

aLogcat (https://play.google.com/store/apps/details?id=org.jtb.alogcat) can do the trick.

Comment 4

[Wesley Johnston (:wesj)]

You can also try the about:logcat extension:

https://people.mozilla.com/~kgupta/aboutlogcat.xpi

save as pdf and post it here. Or I'm in SF. Desk 7123 (by gary's desk).

Comment 5

[Gene Wood [:gene]]

Attachment: aLogcat log during the crash

aLogcat looks like it worked well, attaching the log

Comment 6

[Aaron Train [:aaronmt]]

Bug 861658?

Comment 9

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch

This patch makes Sync use our "shadowed" styles. This way, the color state list is not being looked at android namespace (which 2.2 devices do happily).

Comment 10

[Sriram Ramasubramanian [:sriram]]

Nick,
Github changes please :)

Comment 11

[Sriram Ramasubramanian [:sriram]]

Also please guide me on any other sync file using "android:style/TextAppearance" or "android:style/ [TextView / Button / EditText ] ". Those should be changed too.

Comment 12

[Scoobidiver (away)]

Please attach the patch to bug 861658 unless it can happen under other conditions.

Comment 13

[Nick Alexander :nalexander [he/him]]

(In reply to Sriram Ramasubramanian [:sriram] from comment #11)
> Also please guide me on any other sync file using
> "android:style/TextAppearance" or "android:style/ [TextView / Button /
> EditText ] ". Those should be changed too.

Hi Sriram, your patch in comment 9 (https://bugzilla.mozilla.org/show_bug.cgi?id=861994#c9) catches the 6 instances I see in res/values/sync_styles.xml.

I see 4 uses in res/values-v11/sync_styles.xml.

1.  Can you verify that these 4 don't need to be updated because of the affected Android version (Gingerbread is < v11)?

Also, we are using a progress bar once:

res/layout/sync_setup_jpake_waiting.xml:21: style="@android:style/Widget.ProgressBar.Horizontal"

2.  Can you verify this doesn't need to be updated?

With these two verifications, r=nalexander and I'll apply to GitHub.  Thanks!

Comment 14

[Sriram Ramasubramanian [:sriram]]

(In reply to Nick Alexander :nalexander from comment #13)
> (In reply to Sriram Ramasubramanian [:sriram] from comment #11)
> > Also please guide me on any other sync file using
> > "android:style/TextAppearance" or "android:style/ [TextView / Button /
> > EditText ] ". Those should be changed too.
> 
> Hi Sriram, your patch in comment 9
> (https://bugzilla.mozilla.org/show_bug.cgi?id=861994#c9) catches the 6
> instances I see in res/values/sync_styles.xml.
> 
> I see 4 uses in res/values-v11/sync_styles.xml.
> 
> 1.  Can you verify that these 4 don't need to be updated because of the
> affected Android version (Gingerbread is < v11)?

That's fine. That is related to ActionBar and it won't have any problem. We aren't overriding ActionBar yet.

> 
> Also, we are using a progress bar once:
> 
> res/layout/sync_setup_jpake_waiting.xml:21:
> style="@android:style/Widget.ProgressBar.Horizontal"
> 
> 2.  Can you verify this doesn't need to be updated?

ProgressBar wouldn't have any text. So that's fine too. Basically anything that uses text is affected in 2.2 (and some .x of 2.3). This won't be a problem.

Comment 15

[Nick Alexander :nalexander [he/him]]

Sriram's patch will fix the problem but exposes some things that are out of date in Android Sync.  Sriram is going to work on removing @id references from resources/values*/styles.xml and then I will integrate that work into Android Sync.  I expect we'll land this fix by EOW.

Comment 16

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch: No id

This moves layout information back to layout files. There is no "id" reference in styles.xml.

Ideally label's text appearance should be made to use a style. However, https://bugzilla.mozilla.org/attachment.cgi?id=738270 does more cleanup in that area. Once this patch lands, I'll refresh the other patch with a change for the label.

Comment 17

[Nick Alexander :nalexander [he/him]]

I'm going to get rnewman's feedback on https://github.com/mozilla-services/android-sync/pull/306 before landing the original patch.  I cancelled the review on the second patch because a) I'm not a Fennec peer and b) it didn't fix all the issues in Android Sync (@drawable and a few @layout updates would be needed).  Should land by EOD tomorrow.

Comment 18

[Nick Alexander :nalexander [he/him]]

Pushed (under the ticket number 861658) as https://hg.mozilla.org/integration/mozilla-inbound/rev/312eba5c02d2.