Title: [116353] trunk
Revision
116353
Author
commit-qu...@webkit.org
Date
2012-05-07 14:30:57 -0700 (Mon, 07 May 2012)

Log Message

Selection Background Color Error
https://bugs.webkit.org/show_bug.cgi?id=80382

Patch by Shezan Baig <shezbaig...@gmail.com> on 2012-05-07
Reviewed by David Hyatt.

Source/WebCore:

Determine the text colors and selection colors before painting the
background behind the text.  This is because when determining whether
to invert the selection background, the selection text color should be
used instead of the regular text color.  With this patch, the selection
text color is passed to 'paintSelection' so that the selection
background can be compared against it, instead of comparing against the
CSSPropertyColor value.

Test: fast/backgrounds/selection-background-color.html

* rendering/InlineTextBox.cpp:
(WebCore::InlineTextBox::paint):
    Pass selection text color to paintSelection

(WebCore::InlineTextBox::paintSelection):
    Use the selection text color instead of CSSPropertyColor

* rendering/InlineTextBox.h:
(InlineTextBox):
    Adjust signature of paintSelection to accept text color

LayoutTests:

Added new test case for selection background color.

* fast/backgrounds/selection-background-color.html: Added.
* fast/backgrounds/selection-background-color-expected.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (116352 => 116353)


--- trunk/LayoutTests/ChangeLog	2012-05-07 20:41:52 UTC (rev 116352)
+++ trunk/LayoutTests/ChangeLog	2012-05-07 21:30:57 UTC (rev 116353)
@@ -1,3 +1,15 @@
+2012-05-07  Shezan Baig  <shezbaig...@gmail.com>
+
+        Selection Background Color Error
+        https://bugs.webkit.org/show_bug.cgi?id=80382
+
+        Reviewed by David Hyatt.
+
+        Added new test case for selection background color.
+
+        * fast/backgrounds/selection-background-color.html: Added.
+        * fast/backgrounds/selection-background-color-expected.html: Added.
+
 2012-05-07  Mikhail Pozdnyakov  <mikhail.pozdnya...@intel.com>
 
         Unreviewed. Rebaseline fast/layers/video-layer.html.

Added: trunk/LayoutTests/fast/backgrounds/selection-background-color-expected.html (0 => 116353)


--- trunk/LayoutTests/fast/backgrounds/selection-background-color-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/backgrounds/selection-background-color-expected.html	2012-05-07 21:30:57 UTC (rev 116353)
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<!--
+Test for: https://bugs.webkit.org/show_bug.cgi?id=80382
+          (Selection Background Color Error)
+
+This bug is caused when the selection background is inverted if it
+is the same as the text color.  However, it should compare against
+the selected text color instead of the non-selected text color.
+
+The alpha component must be non-opaque for the bug to get triggered.
+This is because when the selection background is opaque, it will be
+blended with white, which prevents the inversion.
+-->
+<html>
+<head>
+    <style>
+        #A::selection {
+            background: rgba(0,0,0,0.85);
+            color: white;
+        }
+
+        #B::selection {
+            background: rgba(0,255,0,0.85);
+            color: blue;
+        }
+    </style>
+</head>
+<body>
+    <div id="A">Test passes if the selection background is black</div>
+    <div id="B">Test passes if the selection background is green</div>
+</body>
+    <script>
+        var body = document.getElementsByTagName("body")[0];
+        window.getSelection().selectAllChildren(body);
+    </script>
+</html>

Added: trunk/LayoutTests/fast/backgrounds/selection-background-color.html (0 => 116353)


--- trunk/LayoutTests/fast/backgrounds/selection-background-color.html	                        (rev 0)
+++ trunk/LayoutTests/fast/backgrounds/selection-background-color.html	2012-05-07 21:30:57 UTC (rev 116353)
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<!--
+Test for: https://bugs.webkit.org/show_bug.cgi?id=80382
+          (Selection Background Color Error)
+
+This bug is caused when the selection background is inverted if it
+is the same as the text color.  However, it should compare against
+the selected text color instead of the non-selected text color.
+
+The alpha component must be non-opaque for the bug to get triggered.
+This is because when the selection background is opaque, it will be
+blended with white, which prevents the inversion.
+-->
+<html>
+<head>
+    <style>
+        #A {
+            color: rgba(0,0,0,0.85);
+        }
+        #A::selection {
+            background: rgba(0,0,0,0.85);
+            color: white;
+        }
+
+        #B {
+            color: rgba(0,255,0,0.85);
+        }
+        #B::selection {
+            background: rgba(0,255,0,0.85);
+            color: blue;
+        }
+    </style>
+</head>
+<body>
+    <div id="A">Test passes if the selection background is black</div>
+    <div id="B">Test passes if the selection background is green</div>
+</body>
+    <script>
+        var body = document.getElementsByTagName("body")[0];
+        window.getSelection().selectAllChildren(body);
+    </script>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (116352 => 116353)


--- trunk/Source/WebCore/ChangeLog	2012-05-07 20:41:52 UTC (rev 116352)
+++ trunk/Source/WebCore/ChangeLog	2012-05-07 21:30:57 UTC (rev 116353)
@@ -1,3 +1,31 @@
+2012-05-07  Shezan Baig  <shezbaig...@gmail.com>
+
+        Selection Background Color Error
+        https://bugs.webkit.org/show_bug.cgi?id=80382
+
+        Reviewed by David Hyatt.
+
+        Determine the text colors and selection colors before painting the
+        background behind the text.  This is because when determining whether
+        to invert the selection background, the selection text color should be
+        used instead of the regular text color.  With this patch, the selection
+        text color is passed to 'paintSelection' so that the selection
+        background can be compared against it, instead of comparing against the
+        CSSPropertyColor value.
+
+        Test: fast/backgrounds/selection-background-color.html
+
+        * rendering/InlineTextBox.cpp:
+        (WebCore::InlineTextBox::paint):
+            Pass selection text color to paintSelection
+
+        (WebCore::InlineTextBox::paintSelection):
+            Use the selection text color instead of CSSPropertyColor
+
+        * rendering/InlineTextBox.h:
+        (InlineTextBox):
+            Adjust signature of paintSelection to accept text color
+
 2012-05-07  David Reveman  <reve...@chromium.org>
 
         [Chromium] Use GL_CHROMIUM_command_buffer_query to throttle texture uploads.

Modified: trunk/Source/WebCore/rendering/InlineTextBox.cpp (116352 => 116353)


--- trunk/Source/WebCore/rendering/InlineTextBox.cpp	2012-05-07 20:41:52 UTC (rev 116352)
+++ trunk/Source/WebCore/rendering/InlineTextBox.cpp	2012-05-07 21:30:57 UTC (rev 116353)
@@ -534,46 +534,7 @@
     bool containsComposition = renderer()->node() && renderer()->frame()->editor()->compositionNode() == renderer()->node();
     bool useCustomUnderlines = containsComposition && renderer()->frame()->editor()->compositionUsesCustomUnderlines();
 
-    // Set our font.
-    const Font& font = styleToUse->font();
-
-    FloatPoint textOrigin = FloatPoint(boxOrigin.x(), boxOrigin.y() + font.fontMetrics().ascent());
-
-    if (combinedText)
-        combinedText->adjustTextOrigin(textOrigin, boxRect);
-
-    // 1. Paint backgrounds behind text if needed. Examples of such backgrounds include selection
-    // and composition underlines.
-    if (paintInfo.phase != PaintPhaseSelection && paintInfo.phase != PaintPhaseTextClip && !isPrinting) {
-#if PLATFORM(MAC)
-        // Custom highlighters go behind everything else.
-        if (styleToUse->highlight() != nullAtom && !context->paintingDisabled())
-            paintCustomHighlight(adjustedPaintOffset, styleToUse->highlight());
-#endif
-
-        if (containsComposition && !useCustomUnderlines)
-            paintCompositionBackground(context, boxOrigin, styleToUse, font,
-                renderer()->frame()->editor()->compositionStart(),
-                renderer()->frame()->editor()->compositionEnd());
-
-        paintDocumentMarkers(context, boxOrigin, styleToUse, font, true);
-
-        if (haveSelection && !useCustomUnderlines)
-            paintSelection(context, boxOrigin, styleToUse, font);
-    }
-
-    if (Frame* frame = renderer()->frame()) {
-        if (Page* page = frame->page()) {
-            // FIXME: Right now, InlineTextBoxes never call addRelevantUnpaintedObject() even though they might 
-            // legitimately be unpainted if they are waiting on a slow-loading web font. We should fix that, and
-            // when we do, we will have to account for the fact the InlineTextBoxes do not always have unique
-            // renderers and Page currently relies on each unpainted object having a unique renderer.
-            if (paintInfo.phase == PaintPhaseForeground)
-                page->addRelevantRepaintedObject(renderer(), IntRect(boxOrigin.x(), boxOrigin.y(), logicalWidth(), logicalHeight()));
-        }
-    }
-
-    // 2. Now paint the foreground, including text and decorations like underline/overline (in quirks mode only).
+    // Determine the text colors and selection colors.
     Color textFillColor;
     Color textStrokeColor;
     Color emphasisMarkColor;
@@ -660,6 +621,46 @@
         }
     }
 
+    // Set our font.
+    const Font& font = styleToUse->font();
+
+    FloatPoint textOrigin = FloatPoint(boxOrigin.x(), boxOrigin.y() + font.fontMetrics().ascent());
+
+    if (combinedText)
+        combinedText->adjustTextOrigin(textOrigin, boxRect);
+
+    // 1. Paint backgrounds behind text if needed. Examples of such backgrounds include selection
+    // and composition underlines.
+    if (paintInfo.phase != PaintPhaseSelection && paintInfo.phase != PaintPhaseTextClip && !isPrinting) {
+#if PLATFORM(MAC)
+        // Custom highlighters go behind everything else.
+        if (styleToUse->highlight() != nullAtom && !context->paintingDisabled())
+            paintCustomHighlight(adjustedPaintOffset, styleToUse->highlight());
+#endif
+
+        if (containsComposition && !useCustomUnderlines)
+            paintCompositionBackground(context, boxOrigin, styleToUse, font,
+                renderer()->frame()->editor()->compositionStart(),
+                renderer()->frame()->editor()->compositionEnd());
+
+        paintDocumentMarkers(context, boxOrigin, styleToUse, font, true);
+
+        if (haveSelection && !useCustomUnderlines)
+            paintSelection(context, boxOrigin, styleToUse, font, selectionFillColor);
+    }
+
+    if (Frame* frame = renderer()->frame()) {
+        if (Page* page = frame->page()) {
+            // FIXME: Right now, InlineTextBoxes never call addRelevantUnpaintedObject() even though they might
+            // legitimately be unpainted if they are waiting on a slow-loading web font. We should fix that, and
+            // when we do, we will have to account for the fact the InlineTextBoxes do not always have unique
+            // renderers and Page currently relies on each unpainted object having a unique renderer.
+            if (paintInfo.phase == PaintPhaseForeground)
+                page->addRelevantRepaintedObject(renderer(), IntRect(boxOrigin.x(), boxOrigin.y(), logicalWidth(), logicalHeight()));
+        }
+    }
+
+    // 2. Now paint the foreground, including text and decorations like underline/overline (in quirks mode only).
     int length = m_len;
     int maximumLength;
     const UChar* characters;
@@ -806,7 +807,7 @@
     ePos = min(endPos - m_start, (int)m_len);
 }
 
-void InlineTextBox::paintSelection(GraphicsContext* context, const FloatPoint& boxOrigin, RenderStyle* style, const Font& font)
+void InlineTextBox::paintSelection(GraphicsContext* context, const FloatPoint& boxOrigin, RenderStyle* style, const Font& font, Color textColor)
 {
     if (context->paintingDisabled())
         return;
@@ -817,13 +818,12 @@
     if (sPos >= ePos)
         return;
 
-    Color textColor = style->visitedDependentColor(CSSPropertyColor);
     Color c = renderer()->selectionBackgroundColor();
     if (!c.isValid() || c.alpha() == 0)
         return;
 
     // If the text color ends up being the same as the selection background, invert the selection
-    // background.  This should basically never happen, since the selection has transparency.
+    // background.
     if (textColor == c)
         c = Color(0xff - c.red(), 0xff - c.green(), 0xff - c.blue());
 

Modified: trunk/Source/WebCore/rendering/InlineTextBox.h (116352 => 116353)


--- trunk/Source/WebCore/rendering/InlineTextBox.h	2012-05-07 20:41:52 UTC (rev 116352)
+++ trunk/Source/WebCore/rendering/InlineTextBox.h	2012-05-07 21:30:57 UTC (rev 116353)
@@ -181,7 +181,7 @@
 
 private:
     void paintDecoration(GraphicsContext*, const FloatPoint& boxOrigin, int decoration, const ShadowData*);
-    void paintSelection(GraphicsContext*, const FloatPoint& boxOrigin, RenderStyle*, const Font&);
+    void paintSelection(GraphicsContext*, const FloatPoint& boxOrigin, RenderStyle*, const Font&, Color textColor);
     void paintDocumentMarker(GraphicsContext*, const FloatPoint& boxOrigin, DocumentMarker*, RenderStyle*, const Font&, bool grammar);
     void paintTextMatchMarker(GraphicsContext*, const FloatPoint& boxOrigin, DocumentMarker*, RenderStyle*, const Font&);
     void computeRectForReplacementMarker(DocumentMarker*, RenderStyle*, const Font&);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to