Closed Bug 1532850 Opened 5 years ago Closed 5 years ago

Implement the backend for prefers-color-scheme on Android

Categories

(GeckoView :: General, enhancement, P2)

Unspecified
Android
enhancement

Tracking

(firefox65 wontfix, firefox66 wontfix, firefox67 verified, firefox68 verified)

VERIFIED FIXED
mozilla68
Tracking Status
firefox65 --- wontfix
firefox66 --- wontfix
firefox67 --- verified
firefox68 --- verified

People

(Reporter: hiro, Assigned: hiro)

References

Details

(Keywords: dev-doc-complete, Whiteboard: [geckoview:fenix:m4][qa-triaged])

Attachments

(3 files, 1 obsolete file)

(We can no longer open new bugs for Fennec?)

Anyways, here is a patch for that but I don't have any Android devices that can be confirmed whether this implementation works as expected. And I am pretty sure this patch doesn't work on Fennec, I don't recall where the main App (I mean the class we can get the onConfigurationChanged notification) is in Fennec.

I hope one of the folks in the mobile team will finish this work!

I noticed that one of my Android devices has the capability to change night modes and confirmed that the patch works fine on Fennec. Also, I noticed on the gecko view example app up-to-date value doesn't reflect when the corresponding system setting changes since the app is an independent application it needs to implement onConfigurationChanged. I guess bug 1529972 is for that?

Attachment #9048720 - Attachment is obsolete: true

Actually the function will fail on Android since on xpcshell tests we don't
initialize AndroidBridge so that we can't query the corresponding system
settings.

Priority: -- → P2
Whiteboard: [geckoview:fenix:m3]

Moving to Fenix M4 milestone because the Fenix team is not blocked by this bug.

Whiteboard: [geckoview:fenix:m3] → [geckoview:fenix:m4]
Pushed by hikezoe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d3eed8f55897
Implement the backend for prefers-color-scheme on Android. r=geckoview-reviewers,snorp
Pushed by hikezoe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8feadb1f48f2
Check the return value of LookAndFeel::GetInt() in case of the function failure. r=emilio

Backed out for apilint failure

backout: https://hg.mozilla.org/integration/autoland/rev/2eb9f506158630ee2a1b71faec9dfe86fdf2a48a

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=232827549&repo=autoland&lineNumber=2858

[task 2019-03-09T01:09:07.701Z] 01:09:07 INFO - API compatibility issues
[task 2019-03-09T01:09:07.701Z] 01:09:07 INFO - Error: Method removed or incompatible change
[task 2019-03-09T01:09:07.701Z] 01:09:07 INFO - in method @android.support.annotation.UiThread public void orientationChanged(int)
[task 2019-03-09T01:09:07.701Z] 01:09:07 INFO - in public final class GeckoRuntime implements android.os.Parcelable
[task 2019-03-09T01:09:07.701Z] 01:09:07 INFO - in package org.mozilla.geckoview
[task 2019-03-09T01:09:07.702Z] 01:09:07 INFO - at line 188
[task 2019-03-09T01:09:07.702Z] 01:09:07 INFO - > Task :geckoview:apiDiffWithGeckoBinariesDebug
[task 2019-03-09T01:09:07.702Z] 01:09:07 INFO - --- Existing API
[task 2019-03-09T01:09:07.702Z] 01:09:07 INFO - +++ Local API
[task 2019-03-09T01:09:07.702Z] 01:09:07 INFO - @@ -175,19 +175,19 @@
[task 2019-03-09T01:09:07.702Z] 01:09:07 INFO - }
[task 2019-03-09T01:09:07.703Z] 01:09:07 INFO - public final class GeckoRuntime implements android.os.Parcelable {
[task 2019-03-09T01:09:07.703Z] 01:09:07 INFO - ctor public GeckoRuntime();
[task 2019-03-09T01:09:07.703Z] 01:09:07 INFO - method @android.support.annotation.UiThread public void attachTo(@android.support.annotation.NonNull android.content.Context);
[task 2019-03-09T01:09:07.703Z] 01:09:07 INFO - + method @android.support.annotation.UiThread public void configurationChanged(android.content.res.Configuration);
[task 2019-03-09T01:09:07.703Z] 01:09:07 INFO - method @android.support.annotation.UiThread @android.support.annotation.NonNull public static org.mozilla.geckoview.GeckoRuntime create(@android.support.annotation.NonNull android.content.Context);
[task 2019-03-09T01:09:07.704Z] 01:09:07 INFO - method @android.support.annotation.UiThread @android.support.annotation.NonNull public static org.mozilla.geckoview.GeckoRuntime create(@android.support.annotation.NonNull android.content.Context, @android.support.annotation.NonNull org.mozilla.geckoview.GeckoRuntimeSettings);
[task 2019-03-09T01:09:07.704Z] 01:09:07 INFO - method @android.support.annotation.UiThread @android.support.annotation.NonNull public static synchronized org.mozilla.geckoview.GeckoRuntime getDefault(@android.support.annotation.NonNull android.content.Context);
[task 2019-03-09T01:09:07.704Z] 01:09:07 INFO - method @android.support.annotation.UiThread @android.support.annotation.Nullable public org.mozilla.geckoview.GeckoRuntime.Delegate getDelegate();
[task 2019-03-09T01:09:07.704Z] 01:09:07 INFO - method @android.support.annotation.UiThread @android.support.annotation.Nullable public java.io.File getProfileDir();
[task 2019-03-09T01:09:07.704Z] 01:09:07 INFO - method @android.support.annotation.AnyThread @android.support.annotation.NonNull public org.mozilla.geckoview.GeckoRuntimeSettings getSettings();
[task 2019-03-09T01:09:07.704Z] 01:09:07 INFO - method @android.support.annotation.UiThread @android.support.annotation.NonNull public org.mozilla.geckoview.RuntimeTelemetry getTelemetry();
[task 2019-03-09T01:09:07.704Z] 01:09:07 INFO - method @android.support.annotation.UiThread public void orientationChanged();
[task 2019-03-09T01:09:07.704Z] 01:09:07 INFO - - method @android.support.annotation.UiThread public void orientationChanged(int);
[task 2019-03-09T01:09:07.705Z] 01:09:07 INFO - method @android.support.annotation.AnyThread public void readFromParcel(@android.support.annotation.NonNull android.os.Parcel);
[task 2019-03-09T01:09:07.705Z] 01:09:07 INFO - method @android.support.annotation.UiThread @android.support.annotation.NonNull public org.mozilla.geckoview.GeckoResult<java.lang.Void> registerWebExtension(@android.support.annotation.NonNull org.mozilla.geckoview.WebExtension);
[task 2019-03-09T01:09:07.705Z] 01:09:07 INFO - method @android.support.annotation.UiThread public void setDelegate(@android.support.annotation.Nullable org.mozilla.geckoview.GeckoRuntime.Delegate);
[task 2019-03-09T01:09:07.705Z] 01:09:07 INFO - method @android.support.annotation.AnyThread public void shutdown();
[task 2019-03-09T01:09:07.705Z] 01:09:07 INFO - field public static final java.lang.String ACTION_CRASHED = "org.mozilla.gecko.ACTION_CRASHED";
[task 2019-03-09T01:09:07.705Z] 01:09:07 INFO - > Task :geckoview:apiLintHelpWithGeckoBinariesDebug
[task 2019-03-09T01:09:07.705Z] 01:09:07 INFO - The API has been modified. If the changes look correct, please run
[task 2019-03-09T01:09:07.705Z] 01:09:07 INFO - $ ./gradlew apiUpdateFileWithGeckoBinariesDebug
[task 2019-03-09T01:09:07.706Z] 01:09:07 INFO - to update the API file.
[task 2019-03-09T01:09:07.706Z] 01:09:07 INFO - FAILURE: Build failed with an exception.
[task 2019-03-09T01:09:07.706Z] 01:09:07 INFO - * What went wrong:
[task 2019-03-09T01:09:07.706Z] 01:09:07 INFO - Execution failed for task ':geckoview:apiCompatLintWithGeckoBinariesDebug'.
[task 2019-03-09T01:09:07.706Z] 01:09:07 INFO - > Process 'command 'python'' finished with non-zero exit value 131

Flags: needinfo?(hikezoe)

Thanks Natalia for backing them out. I didn't notice that the API is exposed in public. Now I think it's better to land this on 68 and to add a new API instead of replacing the GeckoRuntime.orientationChanged API. James, any suggestions?

Flags: needinfo?(hikezoe) → needinfo?(snorp)

Anyways, I am going to update the patch with the way of adding the new API and preserving the orientationChanaged.

I don't think I'm following. It doesn't look like you changed orientationChanged? The failure has to do with the API file not matching. Does ./gradlew apiUpdateFileWithGeckoBinariesDebug succeed locally for you?

Flags: needinfo?(snorp)

Sorry for confusion. The previous version of the patch changed orientationChanged.

(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) (he/him) from comment #11)

I don't think I'm following. It doesn't look like you changed orientationChanged? The failure has to do with the API file not matching. Does ./gradlew apiUpdateFileWithGeckoBinariesDebug succeed locally for you?

Yes, now it works fine.

Anyways, I will land the patch in 68 not in 67 for the safety. Leaving NI to me, just in case I forget to land it.

Flags: needinfo?(hikezoe)

67=wontfix because Hiro says he plans to land his prefers-color-scheme patch in 68.

OS: All → Android
Pushed by hikezoe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/439686eac8d6
Check the return value of LookAndFeel::GetInt() in case of the function failure. r=emilio
https://hg.mozilla.org/integration/autoland/rev/d32559ed093f
Implement the backend for prefers-color-scheme on Android. r=geckoview-reviewers,snorp

Backed out 2 changesets (bug 1532850) for Android bustage

Backout: https://hg.mozilla.org/integration/autoland/rev/fd67c9a17d9662a03ce0e23dad0cb8beb27405c6

Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=d32559ed093ff279e1dc777724bf132f242b450d&selectedJob=234720440

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=234720440&repo=autoland&lineNumber=2686

task 2019-03-19T08:38:20.636Z] 08:38:20 INFO - FAILURE: Build failed with an exception.
[task 2019-03-19T08:38:20.636Z] 08:38:20 INFO - * What went wrong:
[task 2019-03-19T08:38:20.636Z] 08:38:20 INFO - Execution failed for task ':geckoview:checkstyleWithGeckoBinariesDebug'.
[task 2019-03-19T08:38:20.636Z] 08:38:20 INFO - > Checkstyle rule violations were found. See the report at: file:///builds/worker/workspace/build/src/obj-firefox/gradle/build/mobile/android/geckoview/reports/checkstyle/withGeckoBinariesDebug.html
[task 2019-03-19T08:38:20.637Z] 08:38:20 INFO - Checkstyle files with violations: 2
[task 2019-03-19T08:38:20.637Z] 08:38:20 INFO - Checkstyle violations by severity: [error:3]
[task 2019-03-19T08:38:20.637Z] 08:38:20 INFO - * Try:
[task 2019-03-19T08:38:20.637Z] 08:38:20 INFO - Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.
[task 2019-03-19T08:38:20.638Z] 08:38:20 INFO - * Get more help at https://help.gradle.org
[task 2019-03-19T08:38:20.638Z] 08:38:20 INFO - Deprecated Gradle features were used in this build, making it incompatible with Gradle 5.0.
[task 2019-03-19T08:38:20.638Z] 08:38:20 INFO - Use '--warning-mode all' to show the individual deprecation warnings.
[task 2019-03-19T08:38:20.638Z] 08:38:20 INFO - See https://docs.gradle.org/4.10.2/userguide/command_line_interface.html#sec:command_line_warnings
[task 2019-03-19T08:38:20.638Z] 08:38:20 INFO - BUILD FAILED in 17s
[task 2019-03-19T08:38:20.638Z] 08:38:20 INFO - 12 actionable tasks: 2 executed, 10 up-to-date
[task 2019-03-19T08:38:21.175Z] 08:38:21 INFO - TinderboxPrint: report<br/><a href="https://queue.taskcluster.net/v1/task/Q7QBcdTDQKOsaqoMJlOBpg/runs/0/artifacts/public/android/checkstyle/checkstyle.html">HTML checkstyle report</a>, visit "Inspect Task" link for details
[task 2019-03-19T08:38:21.176Z] 08:38:21 INFO - TinderboxPrint: report<br/><a href="https://queue.taskcluster.net/v1/task/Q7QBcdTDQKOsaqoMJlOBpg/runs/0/artifacts/public/android/checkstyle/checkstyle.xml">XML checkstyle report</a>, visit "Inspect Task" link for details
[task 2019-03-19T08:38:21.176Z] 08:38:21 INFO - TEST-UNEXPECTED-FAIL | android-checkstyle | Checkstyle rule violations were found. See the report at: https://queue.taskcluster.net/v1/task/Q7QBcdTDQKOsaqoMJlOBpg/runs/0/artifacts/public/android/checkstyle/checkstyle.html
[task 2019-03-19T08:38:21.176Z] 08:38:21 INFO - SUITE-START | android-checkstyle
[task 2019-03-19T08:38:21.176Z] 08:38:21 INFO - TEST-START | /builds/worker/workspace/build/src/mobile/android/base/java/org/mozilla/gecko/ANRReporter.java

Flags: needinfo?(hikezoe)
Pushed by hikezoe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d44836af07e0
Check the return value of LookAndFeel::GetInt() in case of the function failure. r=emilio
https://hg.mozilla.org/integration/autoland/rev/da87a2079285
Implement the backend for prefers-color-scheme on Android. r=geckoview-reviewers,snorp

Backed out 2 changesets (Bug 1532850) for apilint failure on a CLOSED TREE

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=da87a2079285249ebebccc57cb961429dbb6176b

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=234729036&repo=autoland&lineNumber=2764

[task 2019-03-19T09:54:33.696Z] 09:54:33 INFO - ERROR: The api changelog file is out of date. Please update the file at
[task 2019-03-19T09:54:33.696Z] 09:54:33 INFO - /builds/worker/workspace/build/src/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/doc-files/CHANGELOG.md
[task 2019-03-19T09:54:33.696Z] 09:54:33 INFO - and then modify the [api-version] line as following:
[task 2019-03-19T09:54:33.696Z] 09:54:33 INFO - >>>>
[task 2019-03-19T09:54:33.697Z] 09:54:33 INFO - [api-version]: e1330c0e7cfa08420041813f07f24a9389020564
[task 2019-03-19T09:54:33.697Z] 09:54:33 INFO - <<<<
[task 2019-03-19T09:54:33.697Z] 09:54:33 INFO - FAILURE: Build failed with an exception.
[task 2019-03-19T09:54:33.697Z] 09:54:33 INFO - * What went wrong:
[task 2019-03-19T09:54:33.697Z] 09:54:33 INFO - Execution failed for task ':geckoview:apiChangelogCheckWithGeckoBinariesDebug'.
[task 2019-03-19T09:54:33.697Z] 09:54:33 INFO - > Process 'command 'python'' finished with non-zero exit value 10
[task 2019-03-19T09:54:33.697Z] 09:54:33 INFO - * Try:
[task 2019-03-19T09:54:33.697Z] 09:54:33 INFO - Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.
[task 2019-03-19T09:54:33.697Z] 09:54:33 INFO - * Get more help at https://help.gradle.org
[task 2019-03-19T09:54:33.697Z] 09:54:33 INFO - Deprecated Gradle features were used in this build, making it incompatible with Gradle 5.0.
[task 2019-03-19T09:54:33.698Z] 09:54:33 INFO - Use '--warning-mode all' to show the individual deprecation warnings.
[task 2019-03-19T09:54:33.698Z] 09:54:33 INFO - See https://docs.gradle.org/4.10.2/userguide/command_line_interface.html#sec:command_line_warnings
[task 2019-03-19T09:54:33.698Z] 09:54:33 INFO - BUILD FAILED in 11s
[task 2019-03-19T09:54:33.699Z] 09:54:33 INFO - 18 actionable tasks: 5 executed, 13 up-to-date
[task 2019-03-19T09:54:34.210Z] 09:54:34 INFO - SUITE-START | android-api-lint
[task 2019-03-19T09:54:34.210Z] 09:54:34 INFO - SUITE-END | android-api-lint
[task 2019-03-19T09:54:34.226Z] 09:54:34 ERROR - Return code: 1
[task 2019-03-19T09:54:34.226Z] 09:54:34 ERROR - 1 not in success codes: [0]
[task 2019-03-19T09:54:34.226Z] 09:54:34 WARNING - setting return code to 2
[task 2019-03-19T09:54:34.226Z] 09:54:34 FATAL - Halting on failure while running ['/usr/bin/python2.7', 'mach', '--log-no-times', 'android', 'api-lint']
[task 2019-03-19T09:54:34.226Z] 09:54:34 FATAL - Running post_fatal callback...
[task 2019-03-19T09:54:34.226Z] 09:54:34 FATAL - Exiting 2
[task 2019-03-19T09:54:34.226Z] 09:54:34 INFO - [mozharness: 2019-03-19 09:54:34.226691Z] Finished build step (failed)
[task 2019-03-19T09:54:34.226Z] 09:54:34 INFO - Running post-run listener: _parse_build_tests_ccov
[task 2019-03-19T09:54:34.226Z] 09:54:34 INFO - Running post-run listener: _shutdown_sccache
[task 2019-03-19T09:54:34.227Z] 09:54:34 INFO - Running post-run listener: _summarize
[task 2019-03-19T09:54:34.227Z] 09:54:34 ERROR - # TBPL FAILURE #
[task 2019-03-19T09:54:34.227Z] 09:54:34 INFO - [mozharness: 2019-03-19 09:54:34.227106Z] FxDesktopBuild summary:
[task 2019-03-19T09:54:34.227Z] 09:54:34 ERROR - # TBPL FAILURE #
[taskcluster 2019-03-19 09:54:34.562Z] === Task Finished ===
[taskcluster 2019-03-19 09:54:35.630Z] Unsuccessful task run with exit code: 2 completed in 403.606 seconds

Flags: needinfo?(hikezoe)

I hope no more lint failure will appear

Flags: needinfo?(hikezoe)
Backout by ccoroiu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e41306512aa2
Backed out 2 changesets for apilint failure on a CLOSED TREE
Pushed by hikezoe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e36b1516f122
Check the return value of LookAndFeel::GetInt() in case of the function failure. r=emilio
https://hg.mozilla.org/integration/autoland/rev/ae4035f567c5
Implement the backend for prefers-color-scheme on Android. r=geckoview-reviewers,snorp
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Blocks: 1494034

Adding DDN here so we know to set the BCD for FxA differently.

Keywords: dev-doc-needed

Hiro, can we uplift this Android support for prefers-color-scheme to 67 Beta? The Gecko support (bug 1494034) already landed in 67.

67=affected because prefers-color-scheme ("CSS dark mode") is still planned for Fenix MVP.

Flags: needinfo?(hikezoe)

Yeah, it's possible. The patches here don't contain any test cases, but the test case what Emilio added in bug 1532358 runs through this Android implementation as well. One thing I am concerned of is that a patch here modified CHANGELOG.md which is the changelog of the GeckoView API changes. If we uplift the change in 67, it will be inconsistent with the CHANGELOG.md in mozilla-central, but it's not a big problem at all? Chris?

Flags: needinfo?(hikezoe) → needinfo?(cpeterson)

(In reply to Hiroyuki Ikezoe (:hiro) from comment #24)

The patches here don't contain any test cases, but the test case what Emilio added in bug 1532358 runs through this Android implementation as well.

Emilio's bug 1532358 landed in 67 so our test coverage is probably good enough.

One thing I am concerned of is that a patch here modified CHANGELOG.md which is the changelog of the GeckoView API changes. If we uplift the change in 67, it will be inconsistent with the CHANGELOG.md in mozilla-central, but it's not a big problem at all?

I agree that the CHANGELOG.md inconsistency is not a big problem. GeckoView 67's CHANGELOG.md will introduce GeckoRuntime#configurationChanged, even though it will say "v68" instead of "v67". :)

Flags: needinfo?(cpeterson)

Beta/Release Uplift Approval Request

  • Feature/Bug causing the regression: Bug 1532850
  • User impact if declined: prefers-color-scheme media queries are not usable
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: It would be nice that someone tests on Pixel 3.

STR:

  1. Just open the link in bug 1528960 comment 12 and see the text in the content

The corresponding setting is in the Developers Options and the name is Night mode.

  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The stuff in this bug did just connect the Android specific information to the corresponding media queries stuff which we've already landed in firefox 67 in bug 1494034, and as far as I can tell the backend works fine on my Android device (sony H8324).
  • String changes made/needed: None
  • Do you want to request approval of these patches as well?: on
Attachment #9053122 - Flags: approval-mozilla-beta?
Flags: qe-verify?

Note that patches for uplifting are attachment 9048790 [details] and attachment 9053122 [details] [diff] [review].

Since we have steps for QE, I would like this bug to be verified on Nightly before taking the uplift to Beta.

Flags: qe-verify? → qe-verify+
Whiteboard: [geckoview:fenix:m4] → [geckoview:fenix:m4][qa-triaged]

Andrei, could you please verify this? thanks

Flags: needinfo?(andrei.bodea)

Verified as fixed in the latest Nightly build(68.0a1)(28-Mar-2019).
I tested the patch using https://jsfiddle.net/wfcbntz3/show on two devices:

  • Pixel 3 with Android 9 where I've enabled Night Mode in both Display settings and in Developer options
  • Samsung S9 with Android 9 where I've enabled Night Mode in both Display settings and in Developer options
    (Note that setting "Night Mode" only is display settings is not enough. It must also be set in Developer options)

and I can confirm it works as expected.

Thanks,
Andrei

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Flags: needinfo?(andrei.bodea)
Comment on attachment 9053122 [details] [diff] [review]
Implement the backend for prefers-color-scheme on Android for beta (with modification the CHANGELOG.md)

Activate prefers-color-scheme media queries on Android, needed for our Geckoview release, fix verified by QA on Nightly, uplift approved for 67 beta 7, thanks.

(Sheriffs, please note that the patches to uplift are indicated in comment #27)
Attachment #9053122 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Verified as fixed in Beta 67.0b7 build 1 on Pixel 3 XL (Android P).

We've marked this as supported in 67 on FxA:

https://developer.mozilla.org/en-US/docs/Web/CSS/@media/prefers-color-scheme

Marking this as ddc

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

Attachment

General

Created:
Updated:
Size: