Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(792)

Issue 187783002: Fix incorrect indices for emphasis marks drawing in InlineTextBox::paint (Closed)

Created:
6 years, 9 months ago by Dominik Röttsches
Modified:
6 years, 9 months ago
CC:
blink-reviews, bemjb+rendering_chromium.org, dsinclair, zoltan1, eae+blinkwatch, leviw+renderwatch, jchaffraix+rendering, pdr.
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Fix incorrect indices for emphasis marks drawing in InlineTextBox::paint The assertion failure in HarfBuzzShaper is triggered because the TextRun passed to paintTextWithShadows is replaced with objectReplacementCharacterTextRun which only has a length of 1. Previously, we did not have an implementation of emphasis mark drawing for complex text with HarfBuzz, so instead of running into the assertion, those paint calls were not doing anything. The indices have been wrong since the emphasis marks drawing for RenderCombineText elements was fixed in 7565e3bbdccf98450271 (in 2011). I updated the test case to be equivalent to the original fuzz testing report and so that it actually combines text that is marked as combined where no font with narrow digits exists. BUG=348682 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=168592

Patch Set 1 #

Patch Set 2 : Keeping previous FIXME #

Unified diffs Side-by-side diffs Delta from patch set Stats (+21 lines, -9 lines) Patch
M LayoutTests/TestExpectations View 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/fast/text/emphasis-combined-text.html View 2 chunks +4 lines, -3 lines 0 comments Download
M Source/core/rendering/InlineTextBox.cpp View 1 2 chunks +16 lines, -6 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
Dominik Röttsches
6 years, 9 months ago (2014-03-05 14:17:01 UTC) #1
leviw_travelin_and_unemployed
LGTM.
6 years, 9 months ago (2014-03-05 19:23:14 UTC) #2
Dominik Röttsches
The CQ bit was checked by dominik.rottsches@intel.com
6 years, 9 months ago (2014-03-05 20:36:01 UTC) #3
Dominik Röttsches
The CQ bit was unchecked by dominik.rottsches@intel.com
6 years, 9 months ago (2014-03-05 20:36:02 UTC) #4
Dominik Röttsches
The CQ bit was checked by dominik.rottsches@intel.com
6 years, 9 months ago (2014-03-05 22:18:43 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dominik.rottsches@intel.com/187783002/20001
6 years, 9 months ago (2014-03-05 22:19:07 UTC) #6
commit-bot: I haz the power
6 years, 9 months ago (2014-03-06 08:07:53 UTC) #7
Message was sent while issue was closed.
Change committed as 168592

Powered by Google App Engine
This is Rietveld 408576698