Bug 124146 – dir=auto causes spaces to not get collapsed

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: Layout and Rendering

[Derk-Jan Hartman]

See the screenshot, a fragment of the page at https://en.wikipedia.org/w/index.php?title=Wikipedia_talk%3ARequests_for_adminship&action=history&offset=20131111000000 

The part of the line after the arrow, but before the ) parentheses is a span with dir=auto set. As is clearly visible, the text in this inline-block is incorrectly measured causing it to overlap the parentheses. Resetting the implied unicode-bidi: -webkit-isolate; to normal, fixes the problem.

A reduced testcase is included

[Derk-Jan Hartman]

Created attachment 216565 [details]
small test case

[Derk-Jan Hartman]

Oh and using: Version 7.0 (9537.71)

[Derk-Jan Hartman]

Created attachment 216566 [details]
Screenshot

[Aharon (Vladimir) Lanin]

This appears to be the same as https://code.google.com/p/chromium/issues/detail?id=323270, which has been fixed there.

[Derk-Jan Hartman]

Can someone import the fix if this really is already fixed in chromium ?

[Aharon (Vladimir) Lanin]

Please also see https://code.google.com/p/chromium/issues/detail?id=330974 (now fixed there).

[Aharon (Vladimir) Lanin]

Still not fixed in Safari 7.0.2 Mac:
Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_2) AppleWebKit/537.74.9 (KHTML, like Gecko) Version/7.0.2 Safari/537.74.9

Is this a duplicate of bug 109624?

Can someone please import the fix?

[Derk-Jan Hartman]

Still a problem in Safari 9 Seed 1..
So sad....

[Radar WebKit Bug Importer]

<rdar://problem/21626274>

[Myles C. Maxfield]

Created attachment 255958 [details]
Reduction

[Myles C. Maxfield]

Created attachment 255959 [details]
Reduction

[Myles C. Maxfield]

Not related to simple line layout

[Myles C. Maxfield]

The layout box for the "a   b" is getting the correct width, but the bidi run still includes the entire "a   b" string, causing our paint code to naively draw in the collapsed spaces

[Myles C. Maxfield]

We mark <span dir="auto"> as an isolate

[Myles C. Maxfield]

Looks like our midpoint logic in RenderBlockFlow::appendRunsForObject() is failing, possibly due to the isolate. (Midpoints are how we perform space collapsing)

[Myles C. Maxfield]

Wow, yeah, our midpoint handling is completely busted with regard to isolate.

In particular, RenderBlockFlow::appendRunsForObject() assumes that we are iterating in the order of midpoints (aka in logical order, because the midpoints are inserted by our line breaking logic, which is (correctly) insensitive to bidi).

However, constructBidiRunsForSegment() in RenderBlockLineLayout.cpp doesn't iterate in logical order, which is acceptable because of the definition of "isolate" (namely, that isolated elements can be processed out-of-band because they don't require any context)

Overall: our line breaking logic builds up a data structure of midpoints in logical order, and when we process a run of text inside constructBidiRunsForSegment(), we keep a cursor into that data structure and expect it to progress at the same rate that our bidi text processing progresses. However, our bidi text processing doesn't operate in logical order, so when the processing consults the data structure, it's looking in the wrong place inside it.

Fixing this is no simple matter.

[Myles C. Maxfield]

After talking with Zalan, we came up with a pretty simple solution for this. The bidi processing is organized into passes; we can just reset the midpoint cursor for each pass.

[Myles C. Maxfield]

*** This bug has been marked as a duplicate of bug 109624 ***

[Derk-Jan Hartman]

This was marked as a duplicate, but this problem as presented in the reducde testcase still exists in both Version 9.0.3 (11601.4.4) and in nightly builds.

[Derk-Jan Hartman]

My apologies, Spotlight failed me in opening WebKit (picked the website, instead of the app) and I hadn't notice therefore I was not testing with the nightly.

Retested, and it is fixed in the nightlies afterall (but not in 9.0.x).

*** This bug has been marked as a duplicate of bug 109624 ***