New issue
Advanced search Search tips
Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 12
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: ----
Type: ----



Sign in to add a comment

[WPT] -webkit-appearance test in external/wpt/compat failing on Mac

Project Member Reported by monorail...@ecosystem-infra.iam.gserviceaccount.com, Feb 7

Issue description

WPT import https://crrev.com/c/906678 introduced new failures in external/wpt/compat:

List of new failures:
[ Mac-mac10.12 ] external/wpt/compat/webkit-appearance.tentative.html new failing tests: https://crrev.com/c/906678/4/third_party/WebKit/LayoutTests/platform/mac-mac10.12/external/wpt/compat/webkit-appearance.tentative-expected.txt

This import contains upstream changes from 63e67be18169690b680ff6f87dbc7f02e6c8b55a to 141951354c4dc58a5dd7b6394c1b7e15dc8e3e30:
Merge pull request #9315 from stephenmcgruer/webkitAppearance: https://github.com/w3c/web-platform-tests/commit/141951354c4dc58a5dd7b6394c1b7e15dc8e3e30
[css-layout-api][css-paint-api] Generalize paintWorklet test runner to work with layoutWorklet.: https://github.com/w3c/web-platform-tests/commit/0bfaa837553a7b66b84b207e702b6b403ad6d990
Update list of properties whitelisted for first-letter: https://github.com/w3c/web-platform-tests/commit/5ed12a3d878689b79a43870a18d91e46f516e18e
Add compat test for -webkit-appearance: https://github.com/w3c/web-platform-tests/commit/fbd8275feb776e4a8db4a9f88e7206c4f3b9d4c2 [affecting this directory]

 
Owner: smcgruer@chromium.org
Test added in https://github.com/w3c/web-platform-tests/pull/9315.

smcgruer@, I take it this failure on Mac was not known/anticipated? Can you take a look?
Cc: smcgruer@chromium.org
Owner: tkent@chromium.org
Status: Assigned (was: Untriaged)
It looks like this is caused by https://chromium.googlesource.com/chromium/src/+/b856aa2c66ff2cea4af05ecd4eb6fb5c56a91129, which falls back to menulist-button for menulist on OSX only.

I'm not sure what we could do about this, other than fixing webkit-appearance on OSX?

Assigning to tkent, who originally authored the CL that does the fallback.
Cc: tkent@chromium.org
Owner: ----
Status: Available (was: Assigned)
Both of WebKit and Blink have many kind of such fallback behavior, and it's observable with getComputedStyle().  So,

- The test doesn't reflect the reality
- Should Webkit and Blink stop exposing the fallback behavior via getComputerStyle() and adjust the appearance value internally?

Summary: [WPT] -webkit-appearance test in external/wpt/compat failing on Mac (was: [WPT] New failures introduced in external/wpt/compat by import https://crrev.com/c/906678)
Components: Blink>CSS
Owner: smcgruer@chromium.org
Status: Assigned (was: Available)
Back to smcgruer@ with the "The test doesn't reflect the reality" challenge :)

tkent@, are the adjustments made here just affecting the visuals, or do they also change something else about the controls? (I don't know what the difference between -webkit-appearance:menulist and -webkit-appearnce:menulist-button is intended to be.)

Is Blink>CSS the right component for this?
> tkent@, are the adjustments made here just affecting the visuals, or do they also change something else about the controls?

The purpose of the adjustments is to make web authors able to control more CSS properties on form controls, and they affect only element's metrics and painting result.

Example:
  <input type=search style="line-height:3em;"> doesn't have 3em line-height, but <input type=search style="line-height:3em; background: lime"> triggers the adjustment, makes computed -webkit-appearance "none", and makes line-height workable.

I am not a -webkit-appearance expert, or even a -webkit-appearance beginner. I am simply trying to ease the path forward for an existing web-compat issue; the existence (and common use) of -webkit-appearance, and the efforts of other browser vendors to also implement it[0].

In order for this feature to work across the web, it will one day have to be spec'd (most likely in https://github.com/whatwg/compat). I expect that on that day, the fact that -webkit-appearance: menulist (or any value) works differently on one OS than another in Chrome will be considered a Chrome bug. But I could be wrong!

I do not have steps forward for this bug. Possibly the best 'right now' behavior is to remove menulist from the WPT test with a comment referencing this bug, or perhaps we should just leave the test as failing on OSX. Or perhaps someone with more understanding can suggest a more currently interoperable WPT?

[0] Safari/Edge already support a subset of -webkit-appearance. Firefox plans to add full support: https://bugzilla.mozilla.org/show_bug.cgi?id=1368555
I recommend to remove the test.  It's too early to add tests.  We need an agreement on getComputedStyle() behavior.

Project Member

Comment 9 by bugdroid1@chromium.org, Feb 26

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/73c0ec5abceb2d983cbdbf5f3e95e3d1371822df

commit 73c0ec5abceb2d983cbdbf5f3e95e3d1371822df
Author: Kent Tamura <tkent@chromium.org>
Date: Mon Feb 26 15:50:35 2018

Count getComputedStyle() for -webkit-appearance

Now getComputedStyle() for -webkit-appearance returns adjusted values.
See [1].  We'd like to know whether we can remove this behavior or not.

[1] https://github.com/whatwg/compat/issues/6#issuecomment-181715665

Bug:  810162 
Change-Id: Icc17d681d7105d8a77ff07e18b7f270bd6918f01
Reviewed-on: https://chromium-review.googlesource.com/936523
Reviewed-by: Takayoshi Kochi <kochi@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539142}
[modify] https://crrev.com/73c0ec5abceb2d983cbdbf5f3e95e3d1371822df/third_party/WebKit/Source/core/css/CSSComputedStyleDeclaration.cpp
[modify] https://crrev.com/73c0ec5abceb2d983cbdbf5f3e95e3d1371822df/third_party/WebKit/public/platform/web_feature.mojom
[modify] https://crrev.com/73c0ec5abceb2d983cbdbf5f3e95e3d1371822df/tools/metrics/histograms/enums.xml

Project Member

Comment 10 by bugdroid1@chromium.org, Jul 12

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/af6945a93a039f0717fbc6037dd66a1db081fa65

commit af6945a93a039f0717fbc6037dd66a1db081fa65
Author: Stephen McGruer <smcgruer@chromium.org>
Date: Thu Jul 12 13:22:01 2018

Remove webkit-appearance.tentative.html WPT test

This test is not a good indicator of -webkit-appearance support; it only
checks whether computed-value == applied-value for -webkit-appearance
values specified on a vanilla <div>. In reality most values of
-webkit-appearance are not of interest to other UAs (see
https://github.com/whatwg/compat/issues/6), and the behavior is
different on different elements (e.g. <input>).

Since this has caused issues across different platforms on Chrome (see
bug), remove it.

Bug:  810162 
Change-Id: I9d469cb624569f453978f3c56cc180eb07435b5b
Reviewed-on: https://chromium-review.googlesource.com/1134387
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574546}
[delete] https://crrev.com/a32928f26183dce010284f0abcee28508e44a6f8/third_party/WebKit/LayoutTests/external/wpt/compat/webkit-appearance.tentative.html
[delete] https://crrev.com/a32928f26183dce010284f0abcee28508e44a6f8/third_party/WebKit/LayoutTests/platform/mac/external/wpt/compat/webkit-appearance.tentative-expected.txt

Status: Fixed (was: Assigned)

Sign in to add a comment