From ddc54aa6de732f13f6bef13c5aded737d495e636 Mon Sep 17 00:00:00 2001 From: Manuel Rego Casasnovas Date: Mon, 14 Aug 2017 15:29:33 +0000 Subject: [PATCH] Merge r220639 - Composition underline color is always black https://bugs.webkit.org/show_bug.cgi?id=174675 Reviewed by Ryosuke Niwa. Source/WebCore: This patch uses the current color of the text instead of black for the composition underline marker. This makes it visible in the case we have a black/dark background. Test: editing/composition-underline-color.html * editing/CompositionUnderline.h: (WebCore::CompositionUnderline::CompositionUnderline): Added new attribute compositionUnderlineColor. * rendering/InlineTextBox.cpp: (WebCore::InlineTextBox::paintCompositionUnderline): Use the text color if compositionUnderlineColor is TextColor. Source/WebKit: This patch uses the current color of the text instead of black for the composition underline marker. This makes it visible in the case we have a black/dark background. * UIProcess/Cocoa/WebViewImpl.mm: (WebKit::extractUnderlines): If NSUnderlineColorAttributeName is not present use text color for composition underline. (WebKit::WebViewImpl::setMarkedText): Use text color for composition underline in the plain text case. * UIProcess/gtk/InputMethodFilter.cpp: (WebKit::InputMethodFilter::handleKeyboardEventWithCompositionResults): Use text color for composition underline. (WebKit::InputMethodFilter::updatePreedit): Ditto. * WebProcess/WebPage/WebPage.cpp: (WebKit::WebPage::setCompositionForTesting): Ditto. Source/WebKitLegacy/mac: * WebView/WebHTMLView.mm: (extractUnderlines): If NSUnderlineColorAttributeName is not present use text color for composition underline. (-[WebHTMLView setMarkedText:selectedRange:]): Use text color for composition underline in the plain text case. Source/WebKitLegacy/win: * WebView.cpp: (WebView::setCompositionForTesting): Use text color for composition underline. LayoutTests: Added new test to check that the composition underline is using the text color. The test hides the text and the caret, so it only shows the composition underline and checks against an -expected-mismatch that the color of the composition marker is different. * editing/composition-underline-color-expected-mismatch.html: Added. * editing/composition-underline-color.html: Added. --- LayoutTests/ChangeLog | 16 +++++++++ ...ion-underline-color-expected-mismatch.html | 33 +++++++++++++++++++ .../editing/composition-underline-color.html | 33 +++++++++++++++++++ Source/WebCore/ChangeLog | 20 +++++++++++ Source/WebCore/editing/CompositionUnderline.h | 15 ++++++--- Source/WebCore/rendering/InlineTextBox.cpp | 2 +- Source/WebKit/ChangeLog | 23 +++++++++++++ Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm | 11 +++++-- .../UIProcess/gtk/InputMethodFilter.cpp | 4 +-- Source/WebKit/WebProcess/WebPage/WebPage.cpp | 2 +- Source/WebKitLegacy/mac/ChangeLog | 13 ++++++++ .../WebKitLegacy/mac/WebView/WebHTMLView.mm | 15 ++++++--- Source/WebKitLegacy/win/ChangeLog | 11 +++++++ Source/WebKitLegacy/win/WebView.cpp | 2 +- 14 files changed, 183 insertions(+), 17 deletions(-) create mode 100644 LayoutTests/editing/composition-underline-color-expected-mismatch.html create mode 100644 LayoutTests/editing/composition-underline-color.html diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index 78b01cae705c..8a676950bd18 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,19 @@ +2017-08-13 Manuel Rego Casasnovas + + Composition underline color is always black + https://bugs.webkit.org/show_bug.cgi?id=174675 + + Reviewed by Ryosuke Niwa. + + Added new test to check that the composition underline + is using the text color. + The test hides the text and the caret, so it only shows + the composition underline and checks against an -expected-mismatch + that the color of the composition marker is different. + + * editing/composition-underline-color-expected-mismatch.html: Added. + * editing/composition-underline-color.html: Added. + 2017-08-10 Nan Wang Layout test accessibility/press-target-uses-text-descendant-node.html is flaky. diff --git a/LayoutTests/editing/composition-underline-color-expected-mismatch.html b/LayoutTests/editing/composition-underline-color-expected-mismatch.html new file mode 100644 index 000000000000..01fc4d6de113 --- /dev/null +++ b/LayoutTests/editing/composition-underline-color-expected-mismatch.html @@ -0,0 +1,33 @@ + + +
+ +
+
+ diff --git a/LayoutTests/editing/composition-underline-color.html b/LayoutTests/editing/composition-underline-color.html new file mode 100644 index 000000000000..c239eb6571cc --- /dev/null +++ b/LayoutTests/editing/composition-underline-color.html @@ -0,0 +1,33 @@ + + +
+ +
+
+ diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index d0ee0b149905..0c9dd322592b 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,23 @@ +2017-08-13 Manuel Rego Casasnovas + + Composition underline color is always black + https://bugs.webkit.org/show_bug.cgi?id=174675 + + Reviewed by Ryosuke Niwa. + + This patch uses the current color of the text instead of black + for the composition underline marker. + This makes it visible in the case we have a black/dark background. + + Test: editing/composition-underline-color.html + + * editing/CompositionUnderline.h: + (WebCore::CompositionUnderline::CompositionUnderline): + Added new attribute compositionUnderlineColor. + * rendering/InlineTextBox.cpp: + (WebCore::InlineTextBox::paintCompositionUnderline): + Use the text color if compositionUnderlineColor is TextColor. + 2017-08-13 Carlos Garcia Campos [GTK] stop kinetic scrolling when a zero movement is reached diff --git a/Source/WebCore/editing/CompositionUnderline.h b/Source/WebCore/editing/CompositionUnderline.h index 6d42816bf946..4ed020a79159 100644 --- a/Source/WebCore/editing/CompositionUnderline.h +++ b/Source/WebCore/editing/CompositionUnderline.h @@ -29,21 +29,26 @@ namespace WebCore { +enum class CompositionUnderlineColor { GivenColor, TextColor }; + struct CompositionUnderline { + CompositionUnderline() { } - CompositionUnderline(unsigned s, unsigned e, const Color& c, bool t) - : startOffset(s) - , endOffset(e) - , color(c) - , thick(t) + CompositionUnderline(unsigned startOffset, unsigned endOffset, CompositionUnderlineColor compositionUnderlineColor, const Color& color, bool thick) + : startOffset(startOffset) + , endOffset(endOffset) + , compositionUnderlineColor(compositionUnderlineColor) + , color(color) + , thick(thick) { } unsigned startOffset { 0 }; unsigned endOffset { 0 }; + CompositionUnderlineColor compositionUnderlineColor { CompositionUnderlineColor::TextColor }; Color color; bool thick { false }; }; diff --git a/Source/WebCore/rendering/InlineTextBox.cpp b/Source/WebCore/rendering/InlineTextBox.cpp index e30a155f955b..d76fe097ca62 100644 --- a/Source/WebCore/rendering/InlineTextBox.cpp +++ b/Source/WebCore/rendering/InlineTextBox.cpp @@ -1007,7 +1007,7 @@ void InlineTextBox::paintCompositionUnderline(GraphicsContext& context, const Fl start += 1; width -= 2; - context.setStrokeColor(underline.color); + context.setStrokeColor(underline.compositionUnderlineColor == CompositionUnderlineColor::TextColor ? renderer().style().visitedDependentColor(CSSPropertyWebkitTextFillColor) : underline.color); context.setStrokeThickness(lineThickness); context.drawLineForText(FloatPoint(boxOrigin.x() + start, boxOrigin.y() + logicalHeight() - lineThickness), width, renderer().document().printing()); } diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog index b61b6de2e470..47c1412c3095 100644 --- a/Source/WebKit/ChangeLog +++ b/Source/WebKit/ChangeLog @@ -1,3 +1,26 @@ +2017-08-13 Manuel Rego Casasnovas + + Composition underline color is always black + https://bugs.webkit.org/show_bug.cgi?id=174675 + + Reviewed by Ryosuke Niwa. + + This patch uses the current color of the text instead of black + for the composition underline marker. + This makes it visible in the case we have a black/dark background. + + * UIProcess/Cocoa/WebViewImpl.mm: + (WebKit::extractUnderlines): If NSUnderlineColorAttributeName + is not present use text color for composition underline. + (WebKit::WebViewImpl::setMarkedText): Use text color + for composition underline in the plain text case. + * UIProcess/gtk/InputMethodFilter.cpp: + (WebKit::InputMethodFilter::handleKeyboardEventWithCompositionResults): + Use text color for composition underline. + (WebKit::InputMethodFilter::updatePreedit): Ditto. + * WebProcess/WebPage/WebPage.cpp: + (WebKit::WebPage::setCompositionForTesting): Ditto. + 2017-08-13 Adrian Perez de Castro [WPE] Implement WebCore::standardUserAgent() diff --git a/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm b/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm index 5f5726c46cda..a16e70d367a4 100644 --- a/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm +++ b/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm @@ -4265,9 +4265,12 @@ static BOOL fileExists(NSString *path) if (NSNumber *style = [attrs objectForKey:NSUnderlineStyleAttributeName]) { WebCore::Color color = WebCore::Color::black; - if (NSColor *colorAttr = [attrs objectForKey:NSUnderlineColorAttributeName]) + WebCore::CompositionUnderlineColor compositionUnderlineColor = WebCore::CompositionUnderlineColor::TextColor; + if (NSColor *colorAttr = [attrs objectForKey:NSUnderlineColorAttributeName]) { color = WebCore::colorFromNSColor(colorAttr); - result.append(WebCore::CompositionUnderline(range.location, NSMaxRange(range), color, style.intValue > 1)); + compositionUnderlineColor = WebCore::CompositionUnderlineColor::GivenColor; + } + result.append(WebCore::CompositionUnderline(range.location, NSMaxRange(range), compositionUnderlineColor, color, style.intValue > 1)); } i = range.location + range.length; @@ -4599,8 +4602,10 @@ static bool eventKeyCodeIsZeroOrNumLockOrFn(NSEvent *event) // FIXME: We ignore most attributes from the string, so an input method cannot specify e.g. a font or a glyph variation. text = [string string]; underlines = extractUnderlines(string); - } else + } else { text = string; + underlines.append(WebCore::CompositionUnderline(0, [text length], WebCore::CompositionUnderlineColor::TextColor, WebCore::Color::black, false)); + } if (inSecureInputState()) { // In password fields, we only allow ASCII dead keys, and don't allow inline input, matching NSSecureTextInputField. diff --git a/Source/WebKit/UIProcess/gtk/InputMethodFilter.cpp b/Source/WebKit/UIProcess/gtk/InputMethodFilter.cpp index fc098ac67bf5..6ceb0c189437 100644 --- a/Source/WebKit/UIProcess/gtk/InputMethodFilter.cpp +++ b/Source/WebKit/UIProcess/gtk/InputMethodFilter.cpp @@ -153,7 +153,7 @@ void InputMethodFilter::handleKeyboardEventWithCompositionResults(GdkEventKey* e m_page->confirmComposition(m_confirmedComposition, -1, 0); if (resultsToSend & Preedit && !m_preedit.isNull()) { - m_page->setComposition(m_preedit, Vector{ CompositionUnderline(0, m_preedit.length(), Color(1, 1, 1), false) }, + m_page->setComposition(m_preedit, Vector { CompositionUnderline(0, m_preedit.length(), CompositionUnderlineColor::TextColor, Color(Color::black), false) }, m_cursorOffset, m_cursorOffset, 0 /* replacement start */, 0 /* replacement end */); } } @@ -260,7 +260,7 @@ void InputMethodFilter::updatePreedit() } #endif // FIXME: We should parse the PangoAttrList that we get from the IM context here. - m_page->setComposition(m_preedit, Vector{ CompositionUnderline(0, m_preedit.length(), Color(1, 1, 1), false) }, + m_page->setComposition(m_preedit, Vector { CompositionUnderline(0, m_preedit.length(), CompositionUnderlineColor::TextColor, Color(Color::black), false) }, m_cursorOffset, m_cursorOffset, 0 /* replacement start */, 0 /* replacement end */); m_preeditChanged = false; } diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp index 1e40f0cc5803..4c2e7c75ecc2 100644 --- a/Source/WebKit/WebProcess/WebPage/WebPage.cpp +++ b/Source/WebKit/WebProcess/WebPage/WebPage.cpp @@ -4669,7 +4669,7 @@ void WebPage::setCompositionForTesting(const String& compositionString, uint64_t return; Vector underlines; - underlines.append(CompositionUnderline(0, compositionString.length(), Color(Color::black), false)); + underlines.append(CompositionUnderline(0, compositionString.length(), CompositionUnderlineColor::TextColor, Color(Color::black), false)); frame.editor().setComposition(compositionString, underlines, from, from + length); } diff --git a/Source/WebKitLegacy/mac/ChangeLog b/Source/WebKitLegacy/mac/ChangeLog index 1e89aee107e9..734edb331ac9 100644 --- a/Source/WebKitLegacy/mac/ChangeLog +++ b/Source/WebKitLegacy/mac/ChangeLog @@ -1,3 +1,16 @@ +2017-08-13 Manuel Rego Casasnovas + + Composition underline color is always black + https://bugs.webkit.org/show_bug.cgi?id=174675 + + Reviewed by Ryosuke Niwa. + + * WebView/WebHTMLView.mm: + (extractUnderlines): If NSUnderlineColorAttributeName + is not present use text color for composition underline. + (-[WebHTMLView setMarkedText:selectedRange:]): Use text color + for composition underline in the plain text case. + 2017-08-08 Brady Eidson Don't enable default icon loading in WK1 for apps linked against old SDKs. diff --git a/Source/WebKitLegacy/mac/WebView/WebHTMLView.mm b/Source/WebKitLegacy/mac/WebView/WebHTMLView.mm index f94733bbeac3..2d2d6eef7d18 100644 --- a/Source/WebKitLegacy/mac/WebView/WebHTMLView.mm +++ b/Source/WebKitLegacy/mac/WebView/WebHTMLView.mm @@ -6945,9 +6945,12 @@ static void extractUnderlines(NSAttributedString *string, Vector 1)); + compositionUnderlineColor = CompositionUnderlineColor::GivenColor; + } + result.append(CompositionUnderline(range.location, NSMaxRange(range), compositionUnderlineColor, color, [style intValue] > 1)); } i = range.location + range.length; @@ -6997,9 +7000,13 @@ - (void)setMarkedText:(id)string selectedRange:(NSRange)newSelRange replacementRange = NSRangeFromString(rangeString); extractUnderlines(string, underlines); - } else -#endif + } else { text = string; + underlines.append(CompositionUnderline(0, [text length], CompositionUnderlineColor::TextColor, Color::black, false)); + } +#else + text = string; +#endif if (replacementRange.location != NSNotFound) [[self _frame] _selectNSRange:replacementRange]; diff --git a/Source/WebKitLegacy/win/ChangeLog b/Source/WebKitLegacy/win/ChangeLog index 21059985220d..b48f8bba402d 100644 --- a/Source/WebKitLegacy/win/ChangeLog +++ b/Source/WebKitLegacy/win/ChangeLog @@ -1,3 +1,14 @@ +2017-08-13 Manuel Rego Casasnovas + + Composition underline color is always black + https://bugs.webkit.org/show_bug.cgi?id=174675 + + Reviewed by Ryosuke Niwa. + + * WebView.cpp: + (WebView::setCompositionForTesting): Use text color for + composition underline. + 2017-07-25 Said Abou-Hallawa Async image decoding for large images should be disabled after the first time a tile is painted diff --git a/Source/WebKitLegacy/win/WebView.cpp b/Source/WebKitLegacy/win/WebView.cpp index f2ecc22acf1e..66efe23170ef 100644 --- a/Source/WebKitLegacy/win/WebView.cpp +++ b/Source/WebKitLegacy/win/WebView.cpp @@ -7564,7 +7564,7 @@ HRESULT WebView::setCompositionForTesting(_In_ BSTR composition, UINT from, UINT String compositionStr = toString(composition); Vector underlines; - underlines.append(CompositionUnderline(0, compositionStr.length(), Color(Color::black), false)); + underlines.append(CompositionUnderline(0, compositionStr.length(), CompositionUnderlineColor::TextColor, Color(Color::black), false)); frame.editor().setComposition(compositionStr, underlines, from, from + length); return S_OK;