Tidy handling of type=color in HTMLInputElement a bit
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 Nov 2015 06:01:37 +0000 (06:01 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 Nov 2015 06:01:37 +0000 (06:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=150786

Reviewed by Andreas Kling.

* html/ColorInputType.cpp: Fix formatting.
(WebCore::ColorInputType::fallbackValue): Use ASCIILiteral for slightly
better performance.
(WebCore::ColorInputType::sanitizeValue): Use convertToASCIILowercase,
since there is no need for the general purpose Unicode lowercasing here;
those non-ASCII characters aren't allowed by isValidColorString.
(WebCore::ColorInputType::suggestions): Rewrote data list code to remove
peculiarities such as using a null check to terminate the loop instead of
the collection length, calling back to HTMLInputElement just to get the
isValidColorString function called. Also used reserveInitialCapacity and
uncheckedAppend for better memory use in the result vector.
(WebCore::ColorInputType::selectColor): Added.

* html/ColorInputType.h: Made derivation from ColorChooserClient private.
Made most functions private. Added overrides for valueAsColor and selectColor,
now both virtual functions in InputType.

* html/HTMLInputElement.cpp: Removed now-unneeded include of ColorInputType.h.
(WebCore::HTMLInputElement::valueAsColor): Added. Calls through to the InputType.
In a later patch, will be used by accessibility code to get the color so it
does not have to replicate the color parsing logic from this element.
(WebCore::HTMLInputElement::selectColor): Renamed from selectColorInColorChooser,
because the longer name is not clearer. Also made this non-conditional.

* html/HTMLInputElement.h: Added valueAsColor, renamed selectColorInColorChooser
to selectColor and made it available unconditionally.

* html/InputType.cpp:
(WebCore::InputType::valueAsColor): Added. Returns transparent color.
(WebCore::InputType::selectColor): Added. Does nothing by default.

* html/InputType.h: Added virtual valueAsColor and selectColor. Also tidied
up the header a bit and removed unneeded Noncopyable (since this class has
a reference for one of the data members and so is intrinsically not copyable).
Made isColorControl available unconditionally.

* testing/Internals.cpp:
(WebCore::Internals::selectColorInColorChooser): Removed conditionals and
made this call selectColor rather than selectColorInColorChooser.

* testing/Internals.h: Made selectColorInColorChooser unconditional.

* testing/Internals.idl: Made selectColorInColorChooser unconditionally
present. Not important to optimize the test internals class by leaving it
out when INPUT_TYPE_COLOR is not enabled.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@191876 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/html/ColorInputType.cpp
Source/WebCore/html/ColorInputType.h
Source/WebCore/html/HTMLInputElement.cpp
Source/WebCore/html/HTMLInputElement.h
Source/WebCore/html/InputType.cpp
Source/WebCore/html/InputType.h
Source/WebCore/testing/Internals.cpp
Source/WebCore/testing/Internals.h
Source/WebCore/testing/Internals.idl

index 9ba04f9265f98e3e980fbdaf8fcf1d3c523a6508..7548099ddaf58416a574177c5fbd5f03d4bf522d 100644 (file)
@@ -1,3 +1,56 @@
+2015-11-01  Darin Adler  <darin@apple.com>
+
+        Tidy handling of type=color in HTMLInputElement a bit
+        https://bugs.webkit.org/show_bug.cgi?id=150786
+
+        Reviewed by Andreas Kling.
+
+        * html/ColorInputType.cpp: Fix formatting.
+        (WebCore::ColorInputType::fallbackValue): Use ASCIILiteral for slightly
+        better performance.
+        (WebCore::ColorInputType::sanitizeValue): Use convertToASCIILowercase,
+        since there is no need for the general purpose Unicode lowercasing here;
+        those non-ASCII characters aren't allowed by isValidColorString.
+        (WebCore::ColorInputType::suggestions): Rewrote data list code to remove
+        peculiarities such as using a null check to terminate the loop instead of
+        the collection length, calling back to HTMLInputElement just to get the
+        isValidColorString function called. Also used reserveInitialCapacity and
+        uncheckedAppend for better memory use in the result vector.
+        (WebCore::ColorInputType::selectColor): Added.
+
+        * html/ColorInputType.h: Made derivation from ColorChooserClient private.
+        Made most functions private. Added overrides for valueAsColor and selectColor,
+        now both virtual functions in InputType.
+
+        * html/HTMLInputElement.cpp: Removed now-unneeded include of ColorInputType.h.
+        (WebCore::HTMLInputElement::valueAsColor): Added. Calls through to the InputType.
+        In a later patch, will be used by accessibility code to get the color so it
+        does not have to replicate the color parsing logic from this element.
+        (WebCore::HTMLInputElement::selectColor): Renamed from selectColorInColorChooser,
+        because the longer name is not clearer. Also made this non-conditional.
+
+        * html/HTMLInputElement.h: Added valueAsColor, renamed selectColorInColorChooser
+        to selectColor and made it available unconditionally.
+
+        * html/InputType.cpp:
+        (WebCore::InputType::valueAsColor): Added. Returns transparent color.
+        (WebCore::InputType::selectColor): Added. Does nothing by default.
+
+        * html/InputType.h: Added virtual valueAsColor and selectColor. Also tidied
+        up the header a bit and removed unneeded Noncopyable (since this class has
+        a reference for one of the data members and so is intrinsically not copyable).
+        Made isColorControl available unconditionally.
+
+        * testing/Internals.cpp:
+        (WebCore::Internals::selectColorInColorChooser): Removed conditionals and
+        made this call selectColor rather than selectColorInColorChooser.
+
+        * testing/Internals.h: Made selectColorInColorChooser unconditional.
+
+        * testing/Internals.idl: Made selectColorInColorChooser unconditionally
+        present. Not important to optimize the test internals class by leaving it
+        out when INPUT_TYPE_COLOR is not enabled.
+
 2015-11-01  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         [ES6] Support Generator Syntax
index c5385a04b6778c5ea2d435b439a94bf207bb6662..92c2040f8ec5fe0ec752bd37f351ae54339b3075 100644 (file)
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2010 Google Inc. All rights reserved.
+ * Copyright (C) 2015 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
@@ -29,7 +30,9 @@
  */
 
 #include "config.h"
+
 #if ENABLE(INPUT_TYPE_COLOR)
+
 #include "ColorInputType.h"
 
 #include "CSSPropertyNames.h"
@@ -86,7 +89,7 @@ bool ColorInputType::supportsRequired() const
 
 String ColorInputType::fallbackValue() const
 {
-    return String("#000000");
+    return ASCIILiteral("#000000");
 }
 
 String ColorInputType::sanitizeValue(const String& proposedValue) const
@@ -94,7 +97,7 @@ String ColorInputType::sanitizeValue(const String& proposedValue) const
     if (!isValidColorString(proposedValue))
         return fallbackValue();
 
-    return proposedValue.lower();
+    return proposedValue.convertToASCIILowercase();
 }
 
 Color ColorInputType::valueAsColor() const
@@ -227,22 +230,25 @@ Vector<Color> ColorInputType::suggestions() const
 {
     Vector<Color> suggestions;
 #if ENABLE(DATALIST_ELEMENT)
-    HTMLDataListElement* dataList = element().dataList();
-    if (dataList) {
+    if (auto* dataList = element().dataList()) {
         Ref<HTMLCollection> options = dataList->options();
-        for (unsigned i = 0; HTMLOptionElement* option = downcast<HTMLOptionElement>(options->item(i)); ++i) {
-            if (!element().isValidValue(option->value()))
-                continue;
-            Color color(option->value());
-            if (!color.isValid())
-                continue;
-            suggestions.append(color);
+        unsigned length = options->length();
+        suggestions.reserveInitialCapacity(length);
+        for (unsigned i = 0; i != length; ++i) {
+            auto value = downcast<HTMLOptionElement>(*options->item(i)).value();
+            if (isValidColorString(value))
+                suggestions.uncheckedAppend(Color(value));
         }
     }
 #endif
     return suggestions;
 }
 
+void ColorInputType::selectColor(const Color& color)
+{
+    didChooseColor(color);
+}
+
 } // namespace WebCore
 
 #endif // ENABLE(INPUT_TYPE_COLOR)
index 02f90ba2f77bc6e11c48b7740f4f80992a3b49bf..fe577caf47c30203de73ea94882ce6aadf53cfe1 100644 (file)
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2010 Google Inc. All rights reserved.
+ * Copyright (C) 2015 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
 #define ColorInputType_h
 
 #if ENABLE(INPUT_TYPE_COLOR)
+
 #include "BaseClickableWithKeyInputType.h"
 #include "ColorChooserClient.h"
 
 namespace WebCore {
 
-class ColorInputType final : public BaseClickableWithKeyInputType, public ColorChooserClient {
+class ColorInputType final : public BaseClickableWithKeyInputType, private ColorChooserClient {
 public:
     explicit ColorInputType(HTMLInputElement& element) : BaseClickableWithKeyInputType(element) { }
     virtual ~ColorInputType();
 
-    // ColorChooserClient implementation.
+private:
     virtual void didChooseColor(const Color&) override;
     virtual void didEndChooser() override;
     virtual IntRect elementRectRelativeToRootView() const override;
     virtual Color currentColor() override;
     virtual bool shouldShowSuggestions() const override;
     virtual Vector<Color> suggestions() const override;
-
-private:
     virtual bool isColorControl() const override;
     virtual const AtomicString& formControlType() const override;
     virtual bool supportsRequired() const override;
@@ -63,8 +63,9 @@ private:
     virtual bool shouldRespectListAttribute() override;
     virtual bool typeMismatchFor(const String&) const override;
     virtual bool shouldResetOnDocumentActivation() override;
+    virtual Color valueAsColor() const override;
+    virtual void selectColor(const Color&) override;
 
-    Color valueAsColor() const;
     void endColorChooser();
     void updateColorSwatch();
     HTMLElement* shadowColorSwatch() const;
index 16223913557b9cedbfc048f8a3eeccab9cf113d1..b1f8a83ca3722af87a1d62a39d756d4980450489 100644 (file)
 #include <wtf/MathExtras.h>
 #include <wtf/Ref.h>
 
-#if ENABLE(INPUT_TYPE_COLOR)
-#include "ColorInputType.h"
-#endif
-
 #if ENABLE(TOUCH_EVENTS)
 #include "TouchEvent.h"
 #endif
@@ -1546,15 +1542,16 @@ void HTMLInputElement::requiredAttributeChanged()
     m_inputType->requiredAttributeChanged();
 }
 
-#if ENABLE(INPUT_TYPE_COLOR)
-void HTMLInputElement::selectColorInColorChooser(const Color& color)
+Color HTMLInputElement::valueAsColor() const
 {
-    if (!m_inputType->isColorControl())
-        return;
-    static_cast<ColorInputType*>(m_inputType.get())->didChooseColor(color);
+    return m_inputType->valueAsColor();
 }
-#endif
-    
+
+void HTMLInputElement::selectColor(const Color& color)
+{
+    m_inputType->selectColor(color);
+}
+
 #if ENABLE(DATALIST_ELEMENT)
 HTMLElement* HTMLInputElement::list() const
 {
index 36ba5f95ec45b5ad26f3954f6bb54f9a35e97510..dd2bd4dac39375789f6b75c15a2b83bccec502ae 100644 (file)
@@ -280,10 +280,8 @@ public:
 
     void cacheSelectionInResponseToSetValue(int caretOffset) { cacheSelection(caretOffset, caretOffset, SelectionHasNoDirection); }
 
-#if ENABLE(INPUT_TYPE_COLOR)
-    // For test purposes.
-    WEBCORE_EXPORT void selectColorInColorChooser(const Color&);
-#endif
+    Color valueAsColor() const; // Returns transparent color if not type=color.
+    WEBCORE_EXPORT void selectColor(const Color&); // Does nothing if not type=color. Simulates user selection of color; intended for testing.
 
     String defaultToolTip() const;
 
index 422e88ca107abf5559e836ccf5214bd92f938b60..a9922c14acaa1c02a0b8bfd16b4dfffe998b2129 100644 (file)
@@ -874,12 +874,10 @@ bool InputType::isSteppable() const
     return false;
 }
 
-#if ENABLE(INPUT_TYPE_COLOR)
 bool InputType::isColorControl() const
 {
     return false;
 }
-#endif
 
 bool InputType::shouldRespectHeightAndWidthAttributes()
 {
@@ -1149,4 +1147,13 @@ void InputType::stepUpFromRenderer(int n)
     }
 }
 
+Color InputType::valueAsColor() const
+{
+    return Color::transparent;
+}
+
+void InputType::selectColor(const Color&)
+{
+}
+
 } // namespace WebCore
index 70e89394b06c52ab9c40318053470c17558b8c26..2f65fa3cad953677657d8d220228963ae3a640c4 100644 (file)
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2010 Google Inc. All rights reserved.
- * Copyright (C) 2011, 2014 Apple Inc. All rights reserved.
+ * Copyright (C) 2011, 2014-2015 Apple Inc. All rights reserved.
  * Copyright (C) 2012 Samsung Electronics. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -38,7 +38,6 @@
 #include "StepRange.h"
 #include <wtf/FastMalloc.h>
 #include <wtf/Forward.h>
-#include <wtf/Noncopyable.h>
 #include <wtf/RefPtr.h>
 
 #if PLATFORM(IOS)
@@ -73,7 +72,6 @@ typedef int ExceptionCode;
 // Do not expose instances of InputType and classes derived from it to classes
 // other than HTMLInputElement.
 class InputType {
-    WTF_MAKE_NONCOPYABLE(InputType);
     WTF_MAKE_FAST_ALLOCATED;
 
 public:
@@ -86,7 +84,7 @@ public:
     virtual const AtomicString& formControlType() const = 0;
     virtual bool canChangeFromAnotherType() const;
 
-    // Type query functions
+    // Type query functions.
 
     // Any time we are using one of these functions it's best to refactor
     // to add a virtual function to allow the input type object to do the
@@ -96,6 +94,7 @@ public:
     // scattered code with special cases for various types.
 
     virtual bool isCheckbox() const;
+    virtual bool isColorControl() const;
     virtual bool isDateField() const;
     virtual bool isDateTimeField() const;
     virtual bool isDateTimeLocalField() const;
@@ -119,11 +118,7 @@ public:
     virtual bool isURLField() const;
     virtual bool isWeekField() const;
 
-#if ENABLE(INPUT_TYPE_COLOR)
-    virtual bool isColorControl() const;
-#endif
-
-    // Form value functions
+    // Form value functions.
 
     virtual bool shouldSaveAndRestoreFormControlState() const;
     virtual FormControlState saveFormControlState() const;
@@ -131,7 +126,7 @@ public:
     virtual bool isFormDataAppendable() const;
     virtual bool appendFormData(FormDataList&, bool multipart) const;
 
-    // DOM property functions
+    // DOM property functions.
 
     virtual bool getTypeSpecificValue(String&); // Checked first, before internal storage or the value attribute.
     virtual String fallbackValue() const; // Checked last, if both internal storage and value attribute are missing.
@@ -142,14 +137,11 @@ public:
     virtual void setValueAsDouble(double, TextFieldEventBehavior, ExceptionCode&) const;
     virtual void setValueAsDecimal(const Decimal&, TextFieldEventBehavior, ExceptionCode&) const;
 
-    // Validation functions
+    // Validation functions.
+
     virtual String validationMessage() const;
     virtual bool supportsValidation() const;
     virtual bool typeMismatchFor(const String&) const;
-    // Type check for the current input value. We do nothing for some types
-    // though typeMismatchFor() does something for them because of value
-    // sanitization.
-    virtual bool typeMismatch() const;
     virtual bool supportsRequired() const;
     virtual bool valueMissing(const String&) const;
     virtual bool hasBadInput() const;
@@ -175,11 +167,16 @@ public:
     virtual String localizeValue(const String&) const;
     virtual String visibleValue() const;
     virtual bool isEmptyValue() const;
-    // Returing the null string means "use the default value."
+
+    // Type check for the current input value. We do nothing for some types
+    // though typeMismatchFor() does something for them because of value sanitization.
+    virtual bool typeMismatch() const;
+
+    // Return value of null string means "use the default value".
     // This function must be called only by HTMLInputElement::sanitizeValue().
     virtual String sanitizeValue(const String&) const;
 
-    // Event handlers
+    // Event handlers.
 
     virtual void handleClickEvent(MouseEvent*);
     virtual void handleMouseDownEvent(MouseEvent*);
@@ -197,6 +194,7 @@ public:
 #endif
 
     // Helpers for event handlers.
+
     virtual bool shouldSubmitImplicitly(Event*);
     virtual PassRefPtr<HTMLFormElement> formForSubmission() const;
     virtual bool hasCustomFocusLogic() const;
@@ -208,14 +206,13 @@ public:
     virtual void accessKeyAction(bool sendMouseEvents);
     virtual bool canBeSuccessfulSubmitButton();
     virtual void subtreeHasChanged();
+    virtual void blur();
 
 #if ENABLE(TOUCH_EVENTS)
     virtual bool hasTouchEventHandler() const;
 #endif
 
-    virtual void blur();
-
-    // Shadow tree handling
+    // Shadow tree handling.
 
     virtual void createShadowSubtree();
     virtual void destroyShadowSubtree();
@@ -232,7 +229,8 @@ public:
     virtual HTMLElement* sliderTrackElement() const { return nullptr; }
     virtual HTMLElement* placeholderElement() const;
 
-    // Miscellaneous functions
+    // Miscellaneous functions.
+
     virtual bool rendererIsNeeded();
     virtual RenderPtr<RenderElement> createInputRenderer(Ref<RenderStyle>&&);
     virtual void addSearchResult();
@@ -246,15 +244,7 @@ public:
     virtual bool shouldRespectAlignAttribute();
     virtual FileList* files();
     virtual void setFiles(PassRefPtr<FileList>);
-#if ENABLE(DRAG_SUPPORT)
-    // Should return true if the given DragData has more than one dropped files.
-    virtual bool receiveDroppedFiles(const DragData&);
-#endif
     virtual Icon* icon() const;
-#if PLATFORM(IOS)
-    virtual String displayString() const;
-#endif
-
     virtual bool shouldSendChangeEventAfterCheckedChanged();
     virtual bool canSetValue(const String&);
     virtual bool storesValueSeparateFromAttribute();
@@ -277,11 +267,10 @@ public:
     virtual void capsLockStateMayHaveChanged();
     virtual void updateAutoFillButton();
     virtual String defaultToolTip() const;
-
-#if ENABLE(DATALIST_ELEMENT)
-    virtual void listAttributeTargetChanged();
-    virtual Decimal findClosestTickMarkValue(const Decimal&);
-#endif
+    virtual bool supportsIndeterminateAppearance() const;
+    virtual bool supportsSelectionAPI() const;
+    virtual Color valueAsColor() const;
+    virtual void selectColor(const Color&);
 
     // Parses the specified string for the type, and return
     // the Decimal value for the parsing result if the parsing
@@ -292,7 +281,7 @@ public:
     // Parses the specified string for this InputType, and returns true if it
     // is successfully parsed. An instance pointed by the DateComponents*
     // parameter will have parsed values and be modified even if the parsing
-    // fails. The DateComponents* parameter may be 0.
+    // fails. The DateComponents* parameter may be null.
     virtual bool parseToDateComponents(const String&, DateComponents*) const;
 
     // Create a string representation of the specified Decimal value for the
@@ -300,14 +289,6 @@ public:
     // string. This should not be called for types without valueAsNumber.
     virtual String serialize(const Decimal&) const;
 
-#if PLATFORM(IOS)
-    virtual DateComponents::Type dateType() const;
-#endif
-
-    virtual bool supportsIndeterminateAppearance() const;
-
-    virtual bool supportsSelectionAPI() const;
-
     // Gets width and height of the input element if the type of the
     // element is image. It returns 0 if the element is not image type.
     virtual unsigned height() const;
@@ -315,6 +296,20 @@ public:
 
     void dispatchSimulatedClickIfActive(KeyboardEvent*) const;
 
+#if ENABLE(DATALIST_ELEMENT)
+    virtual void listAttributeTargetChanged();
+    virtual Decimal findClosestTickMarkValue(const Decimal&);
+#endif
+
+#if ENABLE(DRAG_SUPPORT)
+    virtual bool receiveDroppedFiles(const DragData&);
+#endif
+
+#if PLATFORM(IOS)
+    virtual DateComponents::Type dateType() const;
+    virtual String displayString() const;
+#endif
+
 protected:
     explicit InputType(HTMLInputElement& element) : m_element(element) { }
     HTMLInputElement& element() const { return m_element; }
@@ -330,4 +325,5 @@ private:
 };
 
 } // namespace WebCore
+
 #endif
index 0bc85727b7d666bb00e360d7205fe80c231f89eb..77ace2d7b648daacba778ac4dde107ea13b2e3f6 100644 (file)
@@ -886,15 +886,13 @@ String Internals::visiblePlaceholder(Element* element)
     return String();
 }
 
-#if ENABLE(INPUT_TYPE_COLOR)
 void Internals::selectColorInColorChooser(Element* element, const String& colorValue)
 {
     if (!is<HTMLInputElement>(element))
         return;
     auto& inputElement = downcast<HTMLInputElement>(*element);
-    inputElement.selectColorInColorChooser(Color(colorValue));
+    inputElement.selectColor(Color(colorValue));
 }
-#endif
 
 Vector<String> Internals::formControlStateOfPreviousHistoryItem(ExceptionCode& ec)
 {
index 06b829529bbae930f8a52efc71bb22cc0cf3795d..97a2b9fb3846f8b60371f0ab7cec5104c9baa8f7 100644 (file)
@@ -142,9 +142,7 @@ public:
     bool attached(Node*, ExceptionCode&);
 
     String visiblePlaceholder(Element*);
-#if ENABLE(INPUT_TYPE_COLOR)
     void selectColorInColorChooser(Element*, const String& colorValue);
-#endif
     Vector<String> formControlStateOfPreviousHistoryItem(ExceptionCode&);
     void setFormControlStateOfPreviousHistoryItem(const Vector<String>&, ExceptionCode&);
 
index ba80cc3a06c94ee1d825d729e05a7168288c5977..af9c1f4f08ebc40ef78fccfd55d94f13325624b6 100644 (file)
@@ -115,9 +115,7 @@ enum MediaControlEvent {
     [RaisesException] boolean attached(Node node);
 
     DOMString visiblePlaceholder(Element element);
-#if defined(ENABLE_INPUT_TYPE_COLOR) && ENABLE_INPUT_TYPE_COLOR
     void selectColorInColorChooser(Element element, DOMString colorValue);
-#endif
     [RaisesException] DOMString[] formControlStateOfPreviousHistoryItem();
     [RaisesException] void setFormControlStateOfPreviousHistoryItem(sequence<DOMString> values);