Bug 75512 – [EFL] Implement the hit test result's bounding rect

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebKit EFL

[Jinwoo Song]

Implement the hit test result to return the bounding rect of the element.

[Jinwoo Song]

Created attachment 121041 [details]
Implement the hit test result's bounding rect

[Ryuan Choi]

Comment on attachment 121041 [details]
Implement the hit test result's bounding rect

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

> Source/WebKit/efl/ewk/ewk_frame.cpp:691
> +    RefPtr<WebCore::Node> innerNonSharedNode = result.innerNonSharedNode();

innerNonSharedNode return WebCore::Node*.

> Source/WebKit/efl/ewk/ewk_frame.h:129
> +    } bounding_box; /**< the bounding rect of the element */

How do you think about using Eina_Rectangle ?

[Jinwoo Song]

(In reply to comment #2)
> (From update of attachment 121041 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=121041&action=review
> 
> > Source/WebKit/efl/ewk/ewk_frame.cpp:691
> > +    RefPtr<WebCore::Node> innerNonSharedNode = result.innerNonSharedNode();
> 
> innerNonSharedNode return WebCore::Node*.

I think there is no problem to use the RefPtr for the innerNonSharedNode. 

> 
> > Source/WebKit/efl/ewk/ewk_frame.h:129
> > +    } bounding_box; /**< the bounding rect of the element */
> 
> How do you think about using Eina_Rectangle ?

That's a good suggestion and I'll fix this.

[Jinwoo Song]

Created attachment 121381 [details]
Use the Eina_Rectangle for the bounding rect instead of struct.

[Jinwoo Song]

Created attachment 121382 [details]
Implement the hit test result's bounding rect

Upload new patch file after reflecting the comment to use the Eina_Rectangle.

[Ryuan Choi]

Comment on attachment 121381 [details]
Use the Eina_Rectangle for the bounding rect instead of struct.

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

> Source/WebKit/efl/ewk/ewk_frame.cpp:696
> +    WebCore::IntRect boundingRect = innerNonSharedNode ? innerNonSharedNode->renderer()->absoluteBoundingBoxRect() : WebCore::IntRect();
> +    hitTest->bounding_box.x = boundingRect.x();
> +    hitTest->bounding_box.y = boundingRect.y();
> +    hitTest->bounding_box.w = boundingRect.width();
> +    hitTest->bounding_box.h = boundingRect.height();

hitTest->bounding_box = innerNonSharedNode ? innerNonSharedNode->renderer()->absoluteBoundingBoxRect() : WebCore::IntRect();

[Jinwoo Song]

(In reply to comment #6)
> (From update of attachment 121381 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=121381&action=review
> 
> > Source/WebKit/efl/ewk/ewk_frame.cpp:696
> > +    WebCore::IntRect boundingRect = innerNonSharedNode ? innerNonSharedNode->renderer()->absoluteBoundingBoxRect() : WebCore::IntRect();
> > +    hitTest->bounding_box.x = boundingRect.x();
> > +    hitTest->bounding_box.y = boundingRect.y();
> > +    hitTest->bounding_box.w = boundingRect.width();
> > +    hitTest->bounding_box.h = boundingRect.height();
> 
> hitTest->bounding_box = innerNonSharedNode ? innerNonSharedNode->renderer()->absoluteBoundingBoxRect() : WebCore::IntRect();

Done.

[Jinwoo Song]

Created attachment 121385 [details]
Patch

[Ryuan Choi]

(In reply to comment #8)
> Created an attachment (id=121385) [details]
> Patch

LGTM.

[Gyuyoung Kim]

Comment on attachment 121385 [details]
Patch

Looks OK too.

[Hajime Morrita]

Comment on attachment 121385 [details]
Patch

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

> Source/WebKit/efl/ewk/ewk_frame.cpp:691
> +    hitTest->bounding_box = result.innerNonSharedNode() ? result.innerNonSharedNode()->renderer()->absoluteBoundingBoxRect() : WebCore::IntRect();

I know you just follow what other ports do. But it is really unfortunate to see this
- layering violation: accessing the render object.
- null check which should be done by HItTestResult.

Could you extract this function as a part of HistTestResult method?

[Jinwoo Song]

(In reply to comment #11)
> (From update of attachment 121385 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=121385&action=review
> 
> > Source/WebKit/efl/ewk/ewk_frame.cpp:691
> > +    hitTest->bounding_box = result.innerNonSharedNode() ? result.innerNonSharedNode()->renderer()->absoluteBoundingBoxRect() : WebCore::IntRect();
> 
> I know you just follow what other ports do. But it is really unfortunate to see this
> - layering violation: accessing the render object.
> - null check which should be done by HItTestResult.
> 
> Could you extract this function as a part of HistTestResult method?

Sure, I'll prepare a patch as you suggested.

[Gyuyoung Kim]

Any update ?

[Michael Catanzaro]

Closing this bug because the EFL port has been removed from trunk.

If you feel this bug applies to a different upstream WebKit port and was closed in error, please either update the title and reopen the bug, or leave a comment to request this.