1353868 - Move Java-side Web App manifest parsing into a separate class

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Web Apps (PWAs), defect, P2)

Description

[Jan Henning [:JanH]]

Code outside of the WebAppActivity itself might want to read manifests, so this should be split out.

Comment 2

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Bug 1353868 - Split out web app manifest handling and fix scope handling

Review commit: https://reviewboard.mozilla.org/r/172970/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/172970/

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8901520 [details]
Bug 1353868 - Split out web app manifest handling and fix scope handling

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/172970/diff/1-2/

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8901520 [details]
Bug 1353868 - Split out web app manifest handling and fix scope handling

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/172970/diff/2-3/

Comment 5

[Eugen Sawin [:esawin]]

Comment on attachment 8901520 [details]
Bug 1353868 - Split out web app manifest handling and fix scope handling

https://reviewboard.mozilla.org/r/172970/#review178756

::: mobile/android/base/java/org/mozilla/gecko/webapps/WebAppActivity.java:188
(Diff revision 3)
>              window.setStatusBarColor(ColorUtil.darken(themeColor, 0.25));
>          }
>      }
>  
> -    private void updateScreenOrientation(JSONObject manifest) {
> -        String orientString = manifest.optString("orientation", null);
> +    private void updateScreenOrientation() {
> +        String orientString = mManifest.getOrientation();

Can be final.

::: mobile/android/base/java/org/mozilla/gecko/webapps/WebAppActivity.java:201
(Diff revision 3)
>  
>          setRequestedOrientation(activityOrientation);
>      }
>  
> -    private void updateDisplayMode(JSONObject manifest) {
> -        String displayMode = manifest.optString("display");
> +    private void updateDisplayMode() {
> +        String displayMode = mManifest.getDisplayMode();

Can be final.

::: mobile/android/base/java/org/mozilla/gecko/webapps/WebAppManifest.java:42
(Diff revision 3)
> +    private Uri mScope;
> +    private Uri mStartUri;
> +    private String mDisplayMode;
> +    private String mOrientation;
> +
> +    public static WebAppManifest fromFile(String url, String path) {

Can be final (holds for the rest of the method arguments in the class).

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a4cb65beefec
Split out web app manifest handling and fix scope handling r=esawin

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/a4cb65beefec