1234934 - ZoomedView: (NullPointerException) Attempt to invoke method a null object reference

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

This seems to happen on startup:

> 12-23 13:00:54.297  10811-10827/org.mozilla.fennec_sebastian E/GeckoPrefsHelper﹕ Handler for preference [ui.zoomedview.simplified] threw exception
>     java.lang.NullPointerException: Attempt to invoke virtual method 'void android.widget.TextView.setVisibility(int)' on a null object reference
>             at org.mozilla.gecko.ZoomedView$4.prefValue(ZoomedView.java:513)
>             at org.mozilla.gecko.PrefsHelper$1.handleMessage(PrefsHelper.java:103)
>             at org.mozilla.gecko.EventDispatcher.dispatchEvent(EventDispatcher.java:325)
>             at org.mozilla.gecko.EventDispatcher.dispatchEvent(EventDispatcher.java:248)
>             at org.mozilla.gecko.GeckoAppShell.handleGeckoMessage(GeckoAppShell.java:2276)
>             at org.mozilla.gecko.mozglue.GeckoLoader.nativeRun(Native Method)
>             at org.mozilla.gecko.GeckoThread.run(GeckoThread.java:437)

Comment 1

[:Margaret Leibovic]

Dominique, any ideas?

Comment 2

[Dominique Vincent [:domivinc]]

Attachment: patch-24122015 1-Bug_1234934___Update_the_zoomed_view_UI_only_when_the_view_inflation_is_completed__r_mcomella.patch

Margaret, the preferences listeners are trying to update the zoomed view UI but onFinishInflate is not yet completed.
I moved all the UI updates in a new method and added a test before the UI updates.

I put Michael as reviewer but if you want to make it I can change the commit message.

Comment 3

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8701870 [details] [diff] [review]
patch-24122015 1-Bug_1234934___Update_the_zoomed_view_UI_only_when_the_view_inflation_is_completed__r_mcomella.patch

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

r+ w/ nits.

::: mobile/android/base/java/org/mozilla/gecko/ZoomedView.java
@@ +505,5 @@
>      }
>  
> +    private void updateUI() {
> +        // onFinishInflate is not yet completed, the update of the UI will be done later
> +        if (changeZoomFactorButton == null)

nit: We always put braces around ifs, even for single statement branches.

@@ +512,5 @@
> +            changeZoomFactorButton.setVisibility(View.INVISIBLE);
> +        } else {
> +            changeZoomFactorButton.setVisibility(View.VISIBLE);
> +        }
> +        setTextInZoomFactorButton(zoomFactor);

Don't we only need to do this if `(!isSimplifiedUI)`?

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Thanks for the quick turn-around time, Dominique!

Comment 5

[Dominique Vincent [:domivinc]]

Attachment: patch-29122015 1-Bug_1234934___Update_the_zoomed_view_UI_only_when_the_view_inflation_is_completed__r_mcomella.patch

All changes done!

Comment 6

[Dominique Vincent [:domivinc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d9b704939524

Comment 7

[Dominique Vincent [:domivinc]]

Michael, the target milestone of this bug should probably be updated to 45 in order to push this change with the other zoomed view changes.

Comment 8

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Dominique Vincent [:domivinc] from comment #7)
> Michael, the target milestone of this bug should probably be updated to 45
> in order to push this change with the other zoomed view changes.

Gotcha – matching 45, when zoomed view started riding the trains (bug 1196146). Thanks for being on top of this. :)

btw, target milestone is which branch the patch initially lands in, while "tracking fennec" is used to mark which version it should land in. We have a few dashboards [1] (essentially just bugzilla queries for these flags) that we check during our Thursday triage meetings to ensure the appropriate patches are uplifted to the correct versions.

[1]: https://wiki.mozilla.org/Mobile/Triage#Android

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/8060449a6dd0

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8060449a6dd0

Comment 11

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Dominique, can you request approval for uplift to Aurora?

Comment 12

[Dominique Vincent [:domivinc]]

Comment on attachment 8702696 [details] [diff] [review]
patch-29122015 1-Bug_1234934___Update_the_zoomed_view_UI_only_when_the_view_inflation_is_completed__r_mcomella.patch

Approval Request Comment
[Feature/regressing bug #]: Zoomed view initialization on startup
[User impact if declined]: Preference settings of the zoomed view not always applied correctly
[Describe test coverage new/current, TreeHerder]: Manual test
[Risks and why]: low, changes are limited to the zoomed view and should not have side-effect on other things.
[String/UUID change made/needed]:None

Comment 13

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8702696 [details] [diff] [review]
patch-29122015 1-Bug_1234934___Update_the_zoomed_view_UI_only_when_the_view_inflation_is_completed__r_mcomella.patch

Fix an error on startup, taking it.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/be4d8e26c2b6