BidiResolver should not append an isolated run if it is in a other line.

BidiResolver should not append a run if the run is in an other line.

This patch is one step forward to make the addition of fake runs less error prone.
 
Additionally changes the name of "end" to endOfLine.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=161387

[igoroliveira, 2013-10-29 23:27:02]

On 2013/10/26 08:27:19, igoroliveira wrote:

The patch is clearly wrong. However BidiResolver uses "end" as end of paragraph
sometimes.

[igoroliveira, 2013-10-30 03:05:33]

On 2013/10/29 23:27:02, igoroliveira wrote:
> On 2013/10/26 08:27:19, igoroliveira wrote:
> 
> The patch is clearly wrong. However BidiResolver uses "end" as end of
paragraph
> sometimes.

Now, the last code looks sane.

[leviw_travelin_and_unemployed, 2013-10-30 17:44:36]

https://codereview.chromium.org/46393003/diff/150001/Source/core/rendering/In...
File Source/core/rendering/InlineIterator.h (right):

https://codereview.chromium.org/46393003/diff/150001/Source/core/rendering/In...
Source/core/rendering/InlineIterator.h:426: bool inEndOfParagraph = m_current ==
end || m_current.atEnd() || (inIsolate() && m_current.m_obj == end.m_obj);
This seems pretty weird. isEndOfLine returning "inEndOfParagraph"?

https://codereview.chromium.org/46393003/diff/150001/Source/platform/text/Bid...
File Source/platform/text/BidiResolver.h (right):

https://codereview.chromium.org/46393003/diff/150001/Source/platform/text/Bid...
Source/platform/text/BidiResolver.h:964: m_runAtEndOfLine = Iterator();
This naming confuses me. A run is denoted by 2 iterators: one for the start and
one for the end. This is an Iterator type, and since it's just one, it can't be
a run.

[igoroliveira, 2013-10-30 23:28:45]

On 2013/10/30 17:44:36, Levi wrote:
>
https://codereview.chromium.org/46393003/diff/150001/Source/core/rendering/In...
> File Source/core/rendering/InlineIterator.h (right):
> 
>
https://codereview.chromium.org/46393003/diff/150001/Source/core/rendering/In...
> Source/core/rendering/InlineIterator.h:426: bool inEndOfParagraph = m_current
==
> end || m_current.atEnd() || (inIsolate() && m_current.m_obj == end.m_obj);
> This seems pretty weird. isEndOfLine returning "inEndOfParagraph"?

urgh left over. Fixed!

> 
>
https://codereview.chromium.org/46393003/diff/150001/Source/platform/text/Bid...
> File Source/platform/text/BidiResolver.h (right):
> 
>
https://codereview.chromium.org/46393003/diff/150001/Source/platform/text/Bid...
> Source/platform/text/BidiResolver.h:964: m_runAtEndOfLine = Iterator();
> This naming confuses me. A run is denoted by 2 iterators: one for the start
and
> one for the end. This is an Iterator type, and since it's just one, it can't
be
> a run.

Yes you are right it is a bad name, it is really hard to find a good name
because the levels of craziness in BidiResolver are too damn high!
I called it  for now "m_endOfRunAtEndOfLine", following the UBA spec.

[leviw_travelin_and_unemployed, 2013-11-01 20:46:02]

LGTM. Sad we don't have a test...

https://codereview.chromium.org/46393003/diff/220001/Source/platform/text/Bid...
File Source/platform/text/BidiResolver.h (right):

https://codereview.chromium.org/46393003/diff/220001/Source/platform/text/Bid...
Source/platform/text/BidiResolver.h:247: Iterator m_endOfLine;
It may be good to put a comment explaining what these mean, since it's not
completely obvious.

[eseidel, 2013-11-05 18:55:46]

lgtm

[commit-bot: I haz the power, 2013-11-05 18:56:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/igor.o@sisa.samsung.com/46393003/270001

[commit-bot: I haz the power, 2013-11-05 21:32:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/igor.o@sisa.samsung.com/46393003/270001

[commit-bot: I haz the power, 2013-11-05 22:17:37]

Change committed as 161387