Bug 44417 – Added WebCore::DocumentMarkerController API which returns vector describing all markers.

• Component: DOM
• Status: RESOLVED WONTFIX

[Kamil Blank]

I need functionality which returns the position of n-th marker so I could be able to show it in browser.

First of all I needed WebCore::DocumentMarkerController functionality which returns a vector of IntRects describing text matches markers.
Unfortunately Webcore::DocumentMarkerController::renderedRectsForMarkers() returns only rects in the visible part of document.
I added new functionality WebCore::DocumentMarkerController::firstRectsForMarkers() which returns needed vector. 
Prefix "first" means that function only return first IntRect for every marker.

[Kamil Blank]

Created attachment 65108 [details]
patch

[Rafael Antognolli]

It seems that this functionality got in with the name DocumentMarkerController::renderedRectsForMarkers in the revision http://trac.webkit.org/changeset/65787. Isn't it the same?

Anyway, the bug 44258 was applied (even without this dependency met), and later a bugfix was done for it in bug 44929. Do you think it's still missing some functionality?

[Rafael Antognolli]

(In reply to comment #2)
> It seems that this functionality got in with the name DocumentMarkerController::renderedRectsForMarkers in the revision http://trac.webkit.org/changeset/65787. Isn't it the same?
> 
> Anyway, the bug 44258 was applied (even without this dependency met), and later a bugfix was done for it in bug 44929. Do you think it's still missing some functionality?

Oops, I just read your first comment again. Sorry, my mistake, please don't consider the previous comment as valid.

[Eric Seidel (no email)]

I don't know much about markers, but lots of folks have worked on them over the years.

[Adam Barth]

Comment on attachment 65108 [details]
patch

This appears to be dead code.  Is there an EFL API to go with this patch?

[Kamil Blank]

Hi Adam,

I was working on functionality which allows WebKit-EFL to mark n-th searched text on page.
There are 2 parts of my work:
1) This patch, involves in WebCore part
2) WebKit-EFL patch which uses functionality from WebCore

As you can see in Blocks section, this patch functionality is used by WebKit-EFL patch:
https://bugs.webkit.org/show_bug.cgi?id=44258

Unfortunately, due to strange (for me) behaviour the second patch landed in repository when WebCore part didn't. Because of natural build-break in this situation EFL patch has been changed so it doesn't cause any issues. Whenever this patch is landed, EFL part will be able to use WebCore functionality again.

[Kamil Blank]

Created attachment 82232 [details]
patch

patch adjusted to new sources

[Kamil Blank]

Could someone look at this patch, please?

[Adam Barth]

This patch still appears to be dead code.  Can you wire it up to the EFL API in the same patch so we can see what how this code will be used?  Ideally, we'd test the code at the same time.

[Kamil Blank]

Created attachment 82241 [details]
patch

added fix for ewk_frame_text_matches_nth_pos_get()

[Kamil Blank]

I added fix for EFL API.

It's just a simple revert of this commit: 3ea24e48e37734f9d04599963c06dd491e916a4d
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@66488 268f45cc-cd09-0410-ab3c-d52691b4dbfc

[Antonio Gomes]

Comment on attachment 82232 [details]
patch

Comments start with capital letter and end with a period (.)

[Antonio Gomes]

Comment on attachment 82241 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=82241&action=review

> Source/WebCore/dom/DocumentMarkerController.cpp:342
> +            WebCore::RenderText* text = static_cast<WebCore::RenderText*>(node->renderer());

You probably do not need "WebCore::"

[Kamil Blank]

Created attachment 82253 [details]
patch

Modified regarding to Antonio comments:
+ removed WebCore:: prefix 
+ comments style

[Kamil Blank]

Hi Guys,

Are there any chances for review?

[Kamil Blank]

Could you please take a look into my patch again, please?

[Eric Seidel (no email)]

Comment on attachment 82253 [details]
patch

What is this for, and why is it different from renderedRects?

[Kamil Blank]

(In reply to comment #17)
> (From update of attachment 82253 [details])
> What is this for, and why is it different from renderedRects?

It's needed to be used by below patch:
https://bugs.webkit.org/show_bug.cgi?id=44258 - due to missing function from this patch currently EFL port uses renderedRectsForMarkers() but should used firstRectsForMarkers() (which is implemented here).

The difference between both functions is that renderedRectsForMarkers() returns only rects in visible part of document but I need all rects from document to be able to return n-th marker.

[Alexey Proskuryakov]

Bug 44258 is marked as fixed, so it's still unclear what this is needed for. The question is what is the user observable that needs to be fixed, and why other ports get away without this function today.

[Kamil Blank]

(In reply to comment #19)
> Bug 44258 is marked as fixed, so it's still unclear what this is needed for. The question is what is the user observable that needs to be fixed, and why other ports get away without this function today.

Dear Alexey,

Please, look below at my previous comment (#6) to this bug - it's an explanation of whole situation:

> I was working on functionality which allows WebKit-EFL to mark n-th searched text on page.
> There are 2 parts of my work:
> 1) This patch, involves in WebCore part
> 2) WebKit-EFL patch which uses functionality from WebCore
> 
> As you can see in Blocks section, this patch functionality is used by WebKit-EFL patch:
> https://bugs.webkit.org/show_bug.cgi?id=44258
> 
> Unfortunately, due to strange (for me) behaviour the second patch landed in repository when WebCore part didn't. Because of natural build-break in this situation EFL patch has been changed so it doesn't cause any issues. Whenever this patch is landed, EFL part will be able to use WebCore functionality again.

[Alexey Proskuryakov]

> I was working on functionality which allows WebKit-EFL to mark n-th searched text on page

Safari does that. Why are any WebCore changes needed when other WebKit based browsers can already do what you need?

[Eric Seidel (no email)]

Comment on attachment 82253 [details]
patch

I believe ethis is not needed.

[Eric Seidel (no email)]

I believe this is not needed.  Please re-open with explanation if it can be shown that the methods the other ports use are insufficient for EFLs needs.