Bug 65307 – Document markers layout issue when highlithing text

• Status: UNCONFIRMED
• Component: WebCore Misc.

[Kamil Blank]

Document markers are too high when searching over the http://bbc.co.uk.

In the attachment you can find screenshots from different launchers:
Chrome (Windows) 12.0.742.122 (search option) 
GtkLauncher (Linux) r91753 (webkit_web_view_mark_text_matches, webkit_web_view_set_highlight_text_matches)
EWebLauncher (Linux) r91753 (ewk_view_text_matches_mark)

I also attached zipped bbc page in case of problems with detecting issue on http://bbc.co.uk

[Kamil Blank]

Created attachment 102234 [details]
[screenshot] GtkLauncher (Linux)

[Kamil Blank]

Created attachment 102235 [details]
[screenshot] EWebLauncher (Linux)

[Kamil Blank]

Created attachment 102237 [details]
[screenshot] Chrome (Windows)

[Kamil Blank]

Created attachment 102238 [details]
zipped bbc.co.uk

[Kamil Blank]

I found that this bug started occur after changeset r71465 (http://trac.webkit.org/changeset/71465)
Bug: https://bugs.webkit.org/show_bug.cgi?id=47237

I've found 2 ways to fix this issue on bbc.co.uk however I'd like to ask for your opinion on these approaches as I'm not too familiar with markers.
Both solutions changes if-statement inside RootInlineBox::selectionTop()

1) To make selectionTop() similar to selectionBottom() by adding negation
diff --git a/Source/WebCore/rendering/RootInlineBox.cpp b/Source/WebCore/rendering/RootInlineBox.cpp
index ecfaaf5..259a692 100644
--- a/Source/WebCore/rendering/RootInlineBox.cpp
+++ b/Source/WebCore/rendering/RootInlineBox.cpp
@@ -403,7 +403,7 @@ LayoutUnit RootInlineBox::selectionTop() const
     if (m_hasAnnotationsBefore)
         selectionTop -= !renderer()->style()->isFlippedLinesWritingMode() ? computeOverAnnotationAdjustment(m_lineTop) : computeUnderAnnotationAdjustment(m_lineTop);
 
-    if (renderer()->style()->isFlippedLinesWritingMode())
+    if (!renderer()->style()->isFlippedLinesWritingMode())
         return selectionTop;
 
     LayoutUnit prevBottom = prevRootBox() ? prevRootBox()->selectionBottom() : block()->borderBefore() + block()->paddingBefore();


2) To add !prevRootBox checking as it was before r71435
diff --git a/Source/WebCore/rendering/RootInlineBox.cpp b/Source/WebCore/rendering/RootInlineBox.cpp
index ecfaaf5..8ad4454 100644
--- a/Source/WebCore/rendering/RootInlineBox.cpp
+++ b/Source/WebCore/rendering/RootInlineBox.cpp
@@ -403,7 +403,7 @@ LayoutUnit RootInlineBox::selectionTop() const
     if (m_hasAnnotationsBefore)
         selectionTop -= !renderer()->style()->isFlippedLinesWritingMode() ? computeOverAnnotationAdjustment(m_lineTop) : computeUnderAnnotationAdjustment(m_lineTop);
 
-    if (renderer()->style()->isFlippedLinesWritingMode())
+    if (renderer()->style()->isFlippedLinesWritingMode() !prevRootBox())
         return selectionTop;
 
     LayoutUnit prevBottom = prevRootBox() ? prevRootBox()->selectionBottom() : block()->borderBefore() + block()->paddingBefore();

[Kamil Blank]

I missed || operator in the 2nd solution. Should be:

2) To add !prevRootBox checking as it was before r71435
diff --git a/Source/WebCore/rendering/RootInlineBox.cpp b/Source/WebCore/rendering/RootInlineBox.cpp
index ecfaaf5..8ad4454 100644
--- a/Source/WebCore/rendering/RootInlineBox.cpp
+++ b/Source/WebCore/rendering/RootInlineBox.cpp
@@ -403,7 +403,7 @@ LayoutUnit RootInlineBox::selectionTop() const
     if (m_hasAnnotationsBefore)
         selectionTop -= !renderer()->style()->isFlippedLinesWritingMode() ? computeOverAnnotationAdjustment(m_lineTop) : computeUnderAnnotationAdjustment(m_lineTop);

-    if (renderer()->style()->isFlippedLinesWritingMode())
+    if (renderer()->style()->isFlippedLinesWritingMode() || !prevRootBox())
         return selectionTop;

     LayoutUnit prevBottom = prevRootBox() ? prevRootBox()->selectionBottom() : block()->borderBefore() + block()->paddingBefore();

[Kamil Blank]

I prepared 2 simple pages showing this issue - it should be easier to detect what is the real problem here:

http://students.mimuw.edu.pl/~kb209232/webkit/65307/test.html
http://students.mimuw.edu.pl/~kb209232/webkit/65307/test_js.html - the same but text is selected by js code

Solution 1) I proposed 2 comments above causes some crashes when running layout tests - please, ignore it.