Bidi-Isolate inlines break layout with collapsed whitespace
[WebKit-https.git] / Source / WebCore / ChangeLog
index b9993a2..e3d147c 100644 (file)
@@ -1,3 +1,88 @@
+2015-07-10  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Bidi-Isolate inlines break layout with collapsed whitespace
+        https://bugs.webkit.org/show_bug.cgi?id=109624
+        <rdar://problem/21752834>
+
+        Reviewed by David Hyatt.
+
+        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):
+
 2015-07-10  Daniel Bates  <dabates@apple.com>
 
         Cleanup: WebCore::Pair class should use RefPtr&& instead of PassRefPtr