Split feature FormControlsVerticalWritingModeSupport by adding FormCo…

…ntrolsVerticalWritingModeTextSupport We would like to ship the Form controls vertical writing mode changes in two parts. * FormControlsVerticalWritingModeSupport will ship for elements select, meter, progress, button and input type not text. Then, we will ship * FormControlsVerticalWritingModeTextSupport will ship for elements textarea and input type email, number, password, search, tel, text, url. This is because text control elements have arrow keys movements and need more key binding changes. Change-Id: I21f6f0ba6a66541a0db6f58b83fc8d4dd7534394 Bug: 484651, 681917 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4763426 Reviewed-by: Daniel Cheng <dcheng@chromium.org> Commit-Queue: Di Zhang <dizhangg@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1188647}
chromium · Aug 26, 2023 · 634ca81 · 634ca81
1 parent 3a17d75
commit 634ca81
Show file tree

Hide file tree

Showing 14 changed files with 138 additions and 35 deletions.
diff --git a/...oid_webview/java/src/org/chromium/android_webview/common/ProductionSupportedFlagList.java b/...oid_webview/java/src/org/chromium/android_webview/common/ProductionSupportedFlagList.java
@@ -490,7 +490,8 @@ private ProductionSupportedFlagList() {}
             Flag.baseFeature(ContentFeatures.PREFETCH_NEW_LIMITS,
                     "Enables new limits policy for SpeculationRules Prefetch."),
             Flag.baseFeature(BlinkFeatures.FORM_CONTROLS_VERTICAL_WRITING_MODE_SUPPORT,
-                    "Enables support for CSS vertical writing mode on form controls"),
+                    "Enables support for CSS vertical writing mode on non-text-based form"
+                            + " controls."),
             Flag.baseFeature(BlinkFeatures.FIX_GESTURE_SCROLL_QUEUING_BUG,
                     "Queues gesture scrolls that do not hit a blocking handler, "
                             + "while handling events that hit a blocking handler instantly"
@@ -513,6 +514,8 @@ private ProductionSupportedFlagList() {}
             Flag.baseFeature(ContentFeatures.NAVIGATION_UPDATES_CHILD_VIEWS_VISIBILITY,
                     "Enables notifying children of the top-most RenderWidgetHostView that they "
                             + "were hidden during a navigation."),
+            Flag.baseFeature(BlinkFeatures.FORM_CONTROLS_VERTICAL_WRITING_MODE_TEXT_SUPPORT,
+                    "Enables support for CSS vertical writing mode on text-based form controls."),
             // Add new commandline switches and features above. The final entry should have a
             // trailing comma for cleaner diffs.
     };

diff --git a/testing/variations/fieldtrial_testing_config.json b/testing/variations/fieldtrial_testing_config.json
@@ -6740,6 +6740,28 @@
             ]
         }
     ],
+    "FormControlsVerticalWritingModeTextSupport": [
+        {
+            "platforms": [
+                "android",
+                "android_webview",
+                "chromeos",
+                "chromeos_lacros",
+                "fuchsia",
+                "linux",
+                "mac",
+                "windows"
+            ],
+            "experiments": [
+                {
+                    "name": "Enabled",
+                    "enable_features": [
+                        "FormControlsVerticalWritingModeTextSupport"
+                    ]
+                }
+            ]
+        }
+    ],
     "GCMAccountTokenReporting": [
         {
             "platforms": [

diff --git a/third_party/blink/public/blink_resources.grd b/third_party/blink/public/blink_resources.grd
@@ -29,6 +29,7 @@
       <include name="IDR_UASTYLE_TRANSITION_CSS" file="../renderer/core/css/transition.css" type="BINDATA" compress="brotli"/>
       <include name="IDR_UASTYLE_TRANSITION_ANIMATIONS_CSS" file="../renderer/core/css/transition_animations.css" type="BINDATA" compress="brotli"/>
       <include name="IDR_UASTYLE_FORM_CONTROLS_NOT_VERTICAL_CSS" file="../renderer/core/css/form_controls_not_vertical.css" type="BINDATA" compress="brotli"/>
+      <include name="IDR_UASTYLE_FORM_CONTROLS_NOT_VERTICAL_CSS_TEXT" file="../renderer/core/css/form_controls_not_vertical_text.css" type="BINDATA" compress="brotli"/>
       <include name="IDR_DOCUMENTXMLTREEVIEWER_CSS" file="../renderer/core/xml/DocumentXMLTreeViewer.css" type="BINDATA" compress="brotli"/>
       <include name="IDR_DOCUMENTXMLTREEVIEWER_JS" file="../renderer/core/xml/DocumentXMLTreeViewer.js" type="BINDATA" compress="brotli"/>
       <include name="IDR_VALIDATION_BUBBLE_ICON" file="../renderer/core/html/forms/resources/input_alert.svg" type="BINDATA" compress="brotli"/>

diff --git a/third_party/blink/renderer/core/css/css_default_style_sheets.cc b/third_party/blink/renderer/core/css/css_default_style_sheets.cc
@@ -128,6 +128,7 @@ void CSSDefaultStyleSheets::PrepareForLeakDetection() {
   selectlist_style_sheet_.Clear();
   marker_style_sheet_.Clear();
   form_controls_not_vertical_style_sheet_.Clear();
+  form_controls_not_vertical_style_text_sheet_.Clear();
   // Recreate the default style sheet to clean up possible SVG resources.
   String default_rules = UncompressResourceAsASCIIString(IDR_UASTYLE_HTML_CSS) +
                          LayoutTheme::GetTheme().ExtraDefaultStyleSheet();
@@ -319,22 +320,35 @@ bool CSSDefaultStyleSheets::EnsureDefaultStyleSheetsForElement(
     changed_default_style = true;
   }
 
-  // TODO(crbug.com/681917): The FormControlsVerticalWritingModeSupport
-  // flag enables vertical writing mode to be used on form controls. When
-  // it is *disabled*, we need to force horizontal writing mode.
+  // TODO(crbug.com/681917, crbug.com/484651): We enable vertical writing mode
+  // on form controls using features FormControlsVerticalWritingModeSupport
+  // and FormControlsVerticalWritingModeTextSupport. When it is *disabled*,
+  // we need to force horizontal writing mode.
+  const auto* input = DynamicTo<HTMLInputElement>(element);
   if (!RuntimeEnabledFeatures::
           FormControlsVerticalWritingModeSupportEnabled() &&
       !form_controls_not_vertical_style_sheet_ &&
       (IsA<HTMLProgressElement>(element) || IsA<HTMLMeterElement>(element) ||
-       IsA<HTMLInputElement>(element) || IsA<HTMLTextAreaElement>(element) ||
-       IsA<HTMLButtonElement>(element) || IsA<HTMLSelectElement>(element))) {
+       IsA<HTMLButtonElement>(element) || IsA<HTMLSelectElement>(element) ||
+       (input && !input->IsTextField()))) {
     form_controls_not_vertical_style_sheet_ =
         ParseUASheet(UncompressResourceAsASCIIString(
             IDR_UASTYLE_FORM_CONTROLS_NOT_VERTICAL_CSS));
     AddRulesToDefaultStyleSheets(form_controls_not_vertical_style_sheet_,
                                  NamespaceType::kHTML);
     changed_default_style = true;
   }
+  if (!RuntimeEnabledFeatures::
+          FormControlsVerticalWritingModeTextSupportEnabled() &&
+      !form_controls_not_vertical_style_text_sheet_ &&
+      (IsA<HTMLTextAreaElement>(element) || (input && input->IsTextField()))) {
+    form_controls_not_vertical_style_text_sheet_ =
+        ParseUASheet(UncompressResourceAsASCIIString(
+            IDR_UASTYLE_FORM_CONTROLS_NOT_VERTICAL_CSS_TEXT));
+    AddRulesToDefaultStyleSheets(form_controls_not_vertical_style_text_sheet_,
+                                 NamespaceType::kHTML);
+    changed_default_style = true;
+  }
 
   DCHECK(!default_html_style_->Features().HasIdsInSelectors());
   return changed_default_style;
@@ -450,6 +464,7 @@ void CSSDefaultStyleSheets::Trace(Visitor* visitor) const {
   visitor->Trace(selectlist_style_sheet_);
   visitor->Trace(marker_style_sheet_);
   visitor->Trace(form_controls_not_vertical_style_sheet_);
+  visitor->Trace(form_controls_not_vertical_style_text_sheet_);
 }
 
 }  // namespace blink
diff --git a/third_party/blink/renderer/core/css/css_default_style_sheets.h b/third_party/blink/renderer/core/css/css_default_style_sheets.h
@@ -97,6 +97,9 @@ class CSSDefaultStyleSheets final
   StyleSheetContents* FormControlsNotVerticalSheet() {
     return form_controls_not_vertical_style_sheet_.Get();
   }
+  StyleSheetContents* FormControlsNotVerticalTextSheet() {
+    return form_controls_not_vertical_style_text_sheet_.Get();
+  }
 
   CORE_EXPORT void PrepareForLeakDetection();
 
@@ -160,6 +163,7 @@ class CSSDefaultStyleSheets final
   Member<StyleSheetContents> marker_style_sheet_;
   Member<StyleSheetContents> forced_colors_style_sheet_;
   Member<StyleSheetContents> form_controls_not_vertical_style_sheet_;
+  Member<StyleSheetContents> form_controls_not_vertical_style_text_sheet_;
 
   std::unique_ptr<UAStyleSheetLoader> media_controls_style_sheet_loader_;
 };

diff --git a/third_party/blink/renderer/core/css/form_controls_not_vertical.css b/third_party/blink/renderer/core/css/form_controls_not_vertical.css
@@ -7,12 +7,9 @@
 
 @namespace "http://www.w3.org/1999/xhtml";
 
-/* TODO(crbug.com/681917): This stylesheet is included only if the
-  FormControlsVerticalWritingModeSupport feature is disabled. This keeps
-  form controls from being rendered with a vertical writing mode in that
-  case. */
-input,
-textarea,
+/* TODO(crbug.com/681917): These form controls don't go vertical if
+   FormControlsVerticalWritingModeSupport is disabled. */
+input:not([type="email" i], [type="number" i], [type="password" i], [type="search" i], [type="tel" i], [type="text" i], [type="url" i]),
 select,
 meter,
 progress,

diff --git a/third_party/blink/renderer/core/css/form_controls_not_vertical_text.css b/third_party/blink/renderer/core/css/form_controls_not_vertical_text.css
@@ -0,0 +1,23 @@
+/*
+ * Copyright 2021 The Chromium Authors
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ *
+ * The default style sheet used to render elements with a `popup`
+ * content attribute (with HTMLPopupAttribute feature enabled).
+ */
+
+ @namespace "http://www.w3.org/1999/xhtml";
+
+ /* TODO(crbug.com/484651): These form controls don't go vertical if
+    FormControlsVerticalWritingModeSupportText is disabled. */
+ textarea,
+ input[type="email" i],
+ input[type="number" i],
+ input[type="password" i],
+ input[type="search" i],
+ input[type="tel" i],
+ input[type="text" i],
+ input[type="url" i] {
+   writing-mode: horizontal-tb !important;
+ }
diff --git a/third_party/blink/renderer/platform/runtime_enabled_features.json5 b/third_party/blink/renderer/platform/runtime_enabled_features.json5
@@ -1881,9 +1881,18 @@
       name: "FormControlsVerticalWritingModeDirectionSupport",
     },
     {
+      // Allow these form controls to have vertical writing mode: select, meter,
+      // progress, button and non-text-based input types.
       name: "FormControlsVerticalWritingModeSupport",
       status: "experimental",
     },
+    {
+      // TODO(crbug.com/484651): Allow text-based form controls to have vertical
+      // writing mode: textarea and input types email, number, password, search,
+      // tel, text, url.
+      name: "FormControlsVerticalWritingModeTextSupport",
+      status: "experimental",
+    },
     {
       name: "FormRelAttribute",
       status: "stable",

diff --git a/third_party/blink/web_tests/TestExpectations b/third_party/blink/web_tests/TestExpectations
@@ -1368,18 +1368,18 @@ crbug.com/1432009 external/wpt/css/css-writing-modes/forms/range-input-appearanc
 
 # These tests are mismatch tests which means they will only pass if the rendered page is not the
 # same between vertical and horizontal writing mode. Failing the tests is equivalent to testing
-# that the appearance is the same. For the virtual version, we disable the feature
-# FormControlsVerticalWritingModeSupport and expect failures (meaning vertical writing mode is
-# definitely disabled).
-crbug.com/1471518 virtual/vertical-form-controls-disabled/external/wpt/css/css-writing-modes/forms/progress-appearance-native-vertical.optional.html [ Failure ]
-crbug.com/1471518 virtual/vertical-form-controls-disabled/external/wpt/css/css-writing-modes/forms/meter-appearance-native-vertical.optional.html [ Failure ]
-crbug.com/1471518 virtual/vertical-form-controls-disabled/external/wpt/css/css-writing-modes/forms/textarea-appearance-native-vlr.optional.html [ Failure ]
-crbug.com/1471518 virtual/vertical-form-controls-disabled/external/wpt/css/css-writing-modes/forms/button-appearance-native-vertical.optional.html [ Failure ]
-crbug.com/1471518 virtual/vertical-form-controls-disabled/external/wpt/css/css-writing-modes/forms/select-multiple-appearance-native-vlr.optional.html [ Failure ]
-crbug.com/1471518 virtual/vertical-form-controls-disabled/external/wpt/css/css-writing-modes/forms/color-input-appearance-native-vertical.optional.html [ Failure ]
-crbug.com/1471518 virtual/vertical-form-controls-disabled/external/wpt/css/css-writing-modes/forms/range-input-appearance-native-vertical.optional.html [ Failure ]
-crbug.com/1471518 virtual/vertical-form-controls-disabled/external/wpt/css/css-writing-modes/forms/text-input-block-size.optional.html [ Failure ]
-crbug.com/1471518 virtual/vertical-form-controls-disabled/external/wpt/css/css-writing-modes/forms/date-input-appearance-native-vertical.optional.html [ Failure ]
+# that the appearance is the same. For the virtual version, we disable the features
+# FormControlsVerticalWritingModeSupport/FormControlsVerticalWritingModeTextSupport and expect
+# failures (meaning vertical writing mode is definitely disabled).
+crbug.com/681917 virtual/vertical-form-controls-disabled/external/wpt/css/css-writing-modes/forms/progress-appearance-native-vertical.optional.html [ Failure ]
+crbug.com/681917 virtual/vertical-form-controls-disabled/external/wpt/css/css-writing-modes/forms/meter-appearance-native-vertical.optional.html [ Failure ]
+crbug.com/681917 virtual/vertical-form-controls-disabled/external/wpt/css/css-writing-modes/forms/button-appearance-native-vertical.optional.html [ Failure ]
+crbug.com/681917 virtual/vertical-form-controls-disabled/external/wpt/css/css-writing-modes/forms/select-multiple-appearance-native-vlr.optional.html [ Failure ]
+crbug.com/681917 virtual/vertical-form-controls-disabled/external/wpt/css/css-writing-modes/forms/color-input-appearance-native-vertical.optional.html [ Failure ]
+crbug.com/681917 virtual/vertical-form-controls-disabled/external/wpt/css/css-writing-modes/forms/range-input-appearance-native-vertical.optional.html [ Failure ]
+crbug.com/681917 virtual/vertical-form-controls-disabled/external/wpt/css/css-writing-modes/forms/date-input-appearance-native-vertical.optional.html [ Failure ]
+crbug.com/484651 virtual/vertical-form-controls-text-disabled/external/wpt/css/css-writing-modes/forms/textarea-appearance-native-vlr.optional.html [ Failure ]
+crbug.com/484651 virtual/vertical-form-controls-text-disabled/external/wpt/css/css-writing-modes/forms/text-input-block-size.optional.html [ Failure ]
 
 ### With LayoutNGFragmentItem enabled
 crbug.com/982194 [ Win10.20h2 ] fast/writing-mode/border-image-vertical-lr.html [ Failure Pass ]

diff --git a/third_party/blink/web_tests/VirtualTestSuites b/third_party/blink/web_tests/VirtualTestSuites
@@ -2471,12 +2471,10 @@
     "bases": [
       "external/wpt/css/css-writing-modes/forms/progress-appearance-native-vertical.optional.html",
       "external/wpt/css/css-writing-modes/forms/meter-appearance-native-vertical.optional.html",
-      "external/wpt/css/css-writing-modes/forms/textarea-appearance-native-vlr.optional.html",
       "external/wpt/css/css-writing-modes/forms/button-appearance-native-vertical.optional.html",
       "external/wpt/css/css-writing-modes/forms/select-multiple-appearance-native-vlr.optional.html",
       "external/wpt/css/css-writing-modes/forms/color-input-appearance-native-vertical.optional.html",
       "external/wpt/css/css-writing-modes/forms/range-input-appearance-native-vertical.optional.html",
-      "external/wpt/css/css-writing-modes/forms/text-input-block-size.optional.html",
       "external/wpt/css/css-writing-modes/forms/date-input-appearance-native-vertical.optional.html",
       "external/wpt/css/css-flexbox/flex-item-compressible-002.html"
     ],
@@ -2485,6 +2483,19 @@
     "owners": ["dizhangg@chromium.org"],
     "expires": "Dec 21, 2023"
   },
+  {
+    "prefix": "vertical-form-controls-text-disabled",
+    "platforms": ["Linux", "Mac", "Win"],
+    "bases": [
+      "external/wpt/css/css-writing-modes/forms/textarea-appearance-native-vlr.optional.html",
+      "external/wpt/css/css-writing-modes/forms/text-input-block-size.optional.html",
+      "external/wpt/css/css-flexbox/flex-item-compressible-002.html"
+    ],
+    "args": ["--disable-features=FormControlsVerticalWritingModeTextSupport",
+             "--disable-threaded-compositing", "--disable-threaded-animation"],
+    "owners": ["dizhangg@chromium.org"],
+    "expires": "Dec 21, 2023"
+  },
   "Suite fot tests that should run with single threaded compositing",
   "(As opposed to threaded compositing, which is the default for web tests)",
   "It uses the field `skip_base_tests` to indicate tests that should not run",

diff --git a/...rm-controls-disabled/external/wpt/css/css-flexbox/flex-item-compressible-002-expected.txt b/...rm-controls-disabled/external/wpt/css/css-flexbox/flex-item-compressible-002-expected.txt
@@ -39,12 +39,7 @@ FAIL .flexbox 10 assert_equals:
       <input type="reset" value="XXXXXXX" class="test2" data-expected-height="140">
     </div>
 height expected 140 but got 100
-FAIL .flexbox 11 assert_equals: 
-<div class="flexbox">
-      <div class="spacer"></div>
-      <input type="text" class="test3" data-expected-height="140">
-    </div>
-height expected 140 but got 100
+PASS .flexbox 11
 FAIL .flexbox 12 assert_equals: 
 <div class="flexbox">
       <div class="spacer"></div>

diff --git a/...rtual/vertical-form-controls-disabled/external/wpt/css/css-writing-modes/forms/README.txt b/...rtual/vertical-form-controls-disabled/external/wpt/css/css-writing-modes/forms/README.txt
diff --git a/third_party/blink/web_tests/virtual/vertical-form-controls-text-disabled/README.md b/third_party/blink/web_tests/virtual/vertical-form-controls-text-disabled/README.md
@@ -0,0 +1,3 @@
+# This suite runs tests with --disable-features=FormControlsVerticalWritingModeTextSupport.
+
+This test suite is to make sure that, when the feature is disabled, form controls for text elements cannot go vertical. This test can be removed when the feature is shipped.
diff --git a/...ntrols-text-disabled/external/wpt/css/css-flexbox/flex-item-compressible-002-expected.txt b/...ntrols-text-disabled/external/wpt/css/css-flexbox/flex-item-compressible-002-expected.txt
@@ -0,0 +1,23 @@
+This is a testharness.js-based test.
+PASS .flexbox 1
+PASS .flexbox 2
+PASS .flexbox 3
+PASS .flexbox 4
+PASS .flexbox 5
+PASS .flexbox 6
+PASS .flexbox 7
+PASS .flexbox 8
+PASS .flexbox 9
+PASS .flexbox 10
+FAIL .flexbox 11 assert_equals: 
+<div class="flexbox">
+      <div class="spacer"></div>
+      <input type="text" class="test3" data-expected-height="140">
+    </div>
+height expected 140 but got 100
+PASS .flexbox 12
+PASS .flexbox 13
+PASS .flexbox 14
+PASS .flexbox 15
+Harness: the test ran to completion.
+