Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Issue 618472 Add WebView quirk setting for 8-digit colors
Starred by 6 users Project Member Reported by aelias@chromium.org, Jun 8 2016 Back to list
Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Feature

Blocked on:
issue 618518

Blocking:
issue 76362



Sign in to add a comment
In M52, Blink added support for #RRGGBBAA color format.  Unfortunately, 8-bit color is supported in Android with a AARRGGBB pattern, which leads to a common mistake within Android WebView app CSS to have such pattern, which was no-op before, but in newer versions of Chrome change instead makes blocks of content entirely transparent.

Android has a standard mechanism for introducing breaking API changes, known as targetSdkVersion.  Apps specify a targetSdkVersion in their manifest which states the highest Android version they have tested against.  It's safe to break apps starting at a particular version because that gives developers have an opportunity to make the necessary fixes, and endusers never see any breakage.

A representative example of how to do this is at http://crbug.com/277157 (https://cs.chromium.org/search/?q=useLegacyBackgroundSizeShorthandBehavior ), which was a similar CSS parsing change.  Because we have already noticed two WebView apps being affected (indicating it's a common mistake), and the symptoms are severe, in my opinion that approach is also called for here.  I've discussed this with sgurun@ and we agree this change should not be shipped to stable channel without a targetSdkVersion quirk, so marking this ReleaseBlock-Stable.

See also: 
Intent to ship discussion: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Y6Q69fwcexo
Inbox app bug: http://b/28977540
Gmail app bug: http://b/29104000
Bug/CL for this CSS feature: http://crbug.com/76362 , https://codereview.chromium.org/1936913002
 
Blockedon: 76362
Comment 2 by boliu@chromium.org, Jun 8 2016
which sdk version does apps have to target, N or O?
this is not known since we cannot talk about android release dates here.
O is the safe version.
Blockedon: -76362 618518
Blocking: 76362
Labels: -M-52 -ReleaseBlock-Stable
I'll disable the feature in issue 618518, so this is no longer blocking.
Comment 6 by samsmlee@google.com, Jun 16 2016
Would it make sense to enable just 4-digit colors?
Also, does this affect RGBA() format at all (e.g rgba(255, 255, 255, 1.0)?
Comment 7 by rbyers@chromium.org, Jun 20 2016
Owner: meade@chromium.org
Talked with Eddy, she said this might make sense for style-dev to own while Noel is out.  Assigning to her for further routing / triage.
Labels: -Type-Bug Type-Feature
Owner: ----
Status: Available
Unassigning since I'm not currently working on this
Alright, should this be put on someone's TODO list?
Cc: meade@chromium.org noel@chromium.org
Owner: rbyers@chromium.org
Probably. Hey Rick - Perhaps this should go on one of the WebView people's queues?
Here's the main place where we configure web settings quirks based on target SDK version. Either someone can write a patch that plumbs this down to wherever it needs to go for this feature (and then we'll take care of making sure it gets documented in the release notes), or you can tell us where it needs to go and we can try to get to it.
https://cs.chromium.org/chromium/src/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromium.java?rcl=0&l=172

The one thing that's tricky is that it needs to test against the *next* android release, whose SDK level is not yet set. I think we're currently planning to add a utility function for this to make it easier; I'll ping people and report back.
Owner: torne@chromium.org
Status: Assigned
torne: This is controlled today via the CSSHexAlphaColorEnabled RuntimeEnabledFeature.  So the quirk should probably boil down to calling RuntimeEnabledFeatures::setCSSHexAlphaColorEnabled(false) when targetting an older SDK (but never explicitly setting it to true - leave that to blink to decide).

There's other places in content where we have Android-specific settings for RuntimeEnabledFeatures, eg: https://cs.chromium.org/chromium/src/content/child/runtime_features.cc?type=cs&sq=package:chromium&rcl=1480681004&l=299

But I don't see any existing plumbing from the above WebViewChromium into RuntimeEnabledFeatures.

So either we should:

1) Add some new plumbing from WebViewChromium down to WebRuntimeFeatures (plus the tiny bit of missing plumbing from WebRuntimeFeatures to RuntimeEnabledFeatures).  I think there'd be some Java work to this, so would probably be best for you guys to drive.

OR

2) Add a new WebSetting (like the other quirks) that can disable support for this feature (independent of the RuntimeEnabledFeature - which we'd remove after shipping).  That would mostly be blink work, so should probably go to meade@'s style team.

I don't have a strong opinion of which is best.  Perhaps the latter is most consistent with what we're doing already (though I haven't looked exhaustively)  WDYT?  aelias?
I somewhat prefer approach 1) I think.  That will make for simpler code in the short term, and there is no particular reason to prefer WebSettings plumbing for targetSdkVersion quirks.  They fit the WebRuntimeFeatures "gradual release" idea fairly well and it may well become the new pattern to use indefinitely-maintained WebRuntimeFeatures for new targetSdkVersion quirks in the future.
Components: -Blink>CSS
Looks like WebView team is next to take action here, removing Blink>CSS.
Sign in to add a comment