Bug 65307

Summary: Document markers layout issue when highlithing text
Product: WebKit Reporter: Kamil Blank <k.blank>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: UNCONFIRMED    
Severity: Normal CC: hyatt, jinwoo7.song, k.blank, l.slachciak, mitz
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Bug Depends on: 47237    
Bug Blocks:    
Attachments:
Description Flags
[screenshot] GtkLauncher (Linux)
none
[screenshot] EWebLauncher (Linux)
none
[screenshot] Chrome (Windows)
none
zipped bbc.co.uk none

Kamil Blank
Reported 2011-07-28 00:40:55 PDT
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
Attachments
[screenshot] GtkLauncher (Linux) (813.47 KB, image/png)
2011-07-28 00:41 PDT, Kamil Blank 		no flags
Details
[screenshot] EWebLauncher (Linux) (874.47 KB, image/png)
2011-07-28 00:42 PDT, Kamil Blank 		no flags
Details
[screenshot] Chrome (Windows) (177.25 KB, image/jpeg)
2011-07-28 00:43 PDT, Kamil Blank 		no flags
Details
zipped bbc.co.uk (606.84 KB, application/zip)
2011-07-28 00:46 PDT, Kamil Blank 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Kamil Blank
Comment 1 2011-07-28 00:41:53 PDT
Created attachment 102234 [details] [screenshot] GtkLauncher (Linux)
Kamil Blank
Comment 2 2011-07-28 00:42:49 PDT
Created attachment 102235 [details] [screenshot] EWebLauncher (Linux)
Kamil Blank
Comment 3 2011-07-28 00:43:32 PDT
Created attachment 102237 [details] [screenshot] Chrome (Windows)
Kamil Blank
Comment 4 2011-07-28 00:46:10 PDT
Created attachment 102238 [details] zipped bbc.co.uk
Kamil Blank
Comment 5 2011-08-09 07:37:45 PDT
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
Comment 6 2011-08-10 06:51:02 PDT
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
Comment 7 2011-08-11 13:26:17 PDT
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.
Note You need to log in before you can comment on or make changes to this bug.