Bidi-Isolate inlines break layout with collapsed whitespace
https://bugs.webkit.org/show_bug.cgi?id=109624
<rdar://problem/21752834>
Reviewed by David Hyatt.
Source/WebCore:
This patch changes the logic in constructBidiRunsForSegment() when it encounters an
isolate. It already has logic to create a BidiResolver for the isolated text;
however, that logic doesn't handle setting up the MidpointState at all.
Specifically, we can set the MidpointState's cursor to point to the context which
we can remember from addPlaceholderRunForIsolatedInline(). This information is
remembered in a HashMap in BidiResolver.
This patch is a partial port of Blink patch
https://src.chromium.org/viewvc/blink?view=rev&revision=159203
Here is some explanatory text regarding how we collapse spaces:
Collapsing whitespace happens in a series of phases. The first phase occurs when
we perform line breaking. Here, we keep track of sequences of whitespace which
should be collapsed, in the form of a vector of pairs of InlineIterators. We put
this knowledge into a MidpointState object.
Then, once we have a line, we run the bidi algorithm on the line (including the
whitespace). As output, the bidi algorithm calls the BidiResolver::appendRun()
callback with two InlineIterators each time it wants to create a run. Because
each renderer that we create has to be owned by exactly one DOM node,
BidiResolver::appendRun() iterates between its two InlineIterator arguments,
calling RenderBlockFlow::appendRunsForObject() on each interstitial DOM node.
This is the function where whitespace collapsing happens. The MidpointState object
keeps a cursor into its remembered whitespace sequences. Here, we simply make a
bidi run for each region in between adjacent whitespace pairs in the MidpointState
object. These bidi runs eventually get turned into leaf InlineBoxes.
The problem is that the BidiResolver::appendRun() callbacks don't occur in
string-order, but the Midpoint InlineIterator pairs are in string-order. In
particular, within a particular isolate, appendRun() gets called in string
order, but callbacks that occur for inner isolates are deferred. This means that
RenderBlockFlow::appendRunsForObject() gets confused when it looks for relevant
whitespace to skip.
Test: fast/text/bidi-isolate-whitespace-collapse.html
* platform/text/BidiResolver.h:
(WebCore::MidpointState::numMidpoints): Returning a const unsigned& is silly.
(WebCore::MidpointState::currentMidpoint): Ditto.
(WebCore::MidpointState::setCurrentMidpoint): The isolated MidpointState object
needs to be able to set its current midpoint to point to the first one inside
the isolate.
(WebCore::MidpointState::decrementNumMidpoints): Renamed from "decrease"
(WebCore::MidpointState::betweenMidpoints): This function is true iff
currentMidpoint() % 2. Instead of keeping a member variable, we can just compute
that.
(WebCore::MidpointState::reset): Deleted.
(WebCore::MidpointState::decreaseNumMidpoints): Deleted.
(WebCore::MidpointState::setBetweenMidpoints): Deleted.
* rendering/InlineIterator.h:
(WebCore::IsolateTracker::addFakeRunIfNecessary): Call
RenderBlockFlow::appendRunsForObject() to keep our MidpointState object in sync
when we pop out of the isolated object. However, we pass in a null run list,
because we don't want to append just yet (that happens when we process the
isolate).
(WebCore::InlineBidiResolver::appendRun): Update for new signature of
appendRunsForObject().
* rendering/RenderBlock.h:
(WebCore::RenderBlock::shouldSkipCreatingRunsForObject): Take a reference instead
of a pointer.
* rendering/RenderBlockFlow.h:
* rendering/RenderBlockLineLayout.cpp:
(WebCore::createRun): Ditto.
(WebCore::RenderBlockFlow::appendRunsForObject): Allow someone passing us a null
BidiRunList. In this case, we will keep the resolver's midpointState() up to date,
but won't actually emit any runs.
(WebCore::notifyResolverToResumeInIsolate): Renamed from setUp.
(WebCore::isolatedResolversMidpointState): Calculate the midpoint state for the
isolated resolver.
(WebCore::setUpResolverToResumeInIsolate): Call isolatedResolversMidpointState().
(WebCore::constructBidiRunsForSegment): Pass in the topResolver, which is
necessary for isolatedResolversMidpointState().
* rendering/line/BreakingContext.h:
(WebCore::checkMidpoints):
LayoutTests:
* fast/inline/crash-when-child-renderer-is-removed-and-line-stays-clean-expected.txt:
* fast/text/bidi-isolate-whitespace-collapse-expected.html: Added.
* fast/text/bidi-isolate-whitespace-collapse.html: Added.
* fast/text/international/embed-bidi-style-in-isolate-crash-expected.txt:
* fast/text/remove-text-node-linebox-not-dirty-crash-expected.txt:
Canonical link: https://commits.webkit.org/164923@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@186686 268f45cc-cd09-0410-ab3c-d52691b4dbfc
2015-07-10 20:39:16 +00:00
|
|
|
<!DOCTYPE html>
|
|
|
|
|
<html>
|
|
|
|
|
<style>
|
|
|
|
|
body > div {
|
|
|
|
|
display: inline-block;
|
|
|
|
|
border: 1px solid black;
|
|
|
|
|
width: 400px;
|
|
|
|
|
font-size: 30px;
|
|
|
|
|
}
|
|
|
|
|
</style>
|
|
|
|
|
<body>
|
|
|
|
|
<p>This test makes sure that whitespace collapsing occurs correctly in the presence of bidi isolates.</p>
|
|
|
|
|
<div id="test">
|
|
|
|
|
<p>123
|
|
|
|
|
<span style="unicode-bidi: -webkit-isolate">456</span> 789</p>
|
|
|
|
|
<p>123 <span style="unicode-bidi: -webkit-isolate">456</span> 789</p>
|
|
|
|
|
<p>123
|
|
|
|
|
<span style="unicode-bidi: -webkit-isolate">4 56
|
|
|
|
|
</span> 789</p>
|
|
|
|
|
<p>123
|
|
|
|
|
<span style="unicode-bidi: -webkit-isolate">
|
|
|
|
|
456
|
|
|
|
|
</span>
|
|
|
|
|
789</p>
|
|
|
|
|
<p>1 2 3
|
|
|
|
|
<span style="unicode-bidi: -webkit-isolate">
|
|
|
|
|
4 5 6
|
|
|
|
|
</span>
|
|
|
|
|
7 8 9 </p>
|
|
|
|
|
</p>
|
|
|
|
|
<p>123
|
|
|
|
|
<span style="unicode-bidi: -webkit-isolate"></span> 789</p>
|
|
|
|
|
<p>123
|
|
|
|
|
<span style="unicode-bidi: -webkit-isolate"> </span> 789</p>
|
|
|
|
|
</div>
|
|
|
|
|
</body></html>
|