Bug 116842 - Move hasNonEmptyBoundingBox from Node to HTMLAnchorElement
Summary: Move hasNonEmptyBoundingBox from Node to HTMLAnchorElement
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Darin Adler
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-05-27 13:43 PDT by Darin Adler
Modified: 2019-02-06 09:18 PST (History)
10 users (show)

See Also:

Attachments
Patch (6.62 KB, patch)
2013-05-27 13:51 PDT, Darin Adler 		no flags Details | Formatted Diff | Diff
Patch (6.55 KB, patch)
2013-05-27 13:51 PDT, Darin Adler 		no flags Details | Formatted Diff | Diff
Patch (5.24 KB, patch)
2013-05-27 16:28 PDT, Darin Adler 		koivisto: review+
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2013-05-27 13:43:56 PDT 
Move hasNonEmptyBoundingBox from Node to RenderBoxModelObject
Comment 1 Darin Adler 2013-05-27 13:51:21 PDT 
Created attachment 203007 [details]
Patch
Comment 2 Darin Adler 2013-05-27 13:51:46 PDT 
Created attachment 203008 [details]
Patch
Comment 3 Antti Koivisto 2013-05-27 16:00:05 PDT 
Comment on attachment 203008 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        Move hasNonEmptyBoundingBox from Node to RenderBoxModelObject

Why there?

> Source/WebCore/html/HTMLAnchorElement.cpp:125
> -    return hasNonEmptyBoundingBox();
> +    RenderBoxModelObject* renderer = renderBoxModelObject();
> +    return renderer && renderer->hasNonEmptyBox();

Unless this is going to have other clients I would just put it to HTMLAnchorElement. No need to bloat RenderBoxModelObject with questionable special case functions like this (any more than Node).

> Source/WebCore/rendering/RenderBoxModelObject.h:178
> +    bool hasNonEmptyBox() const;

isEmpty()?
Comment 4 Darin Adler 2013-05-27 16:18:44 PDT 
Comment on attachment 203008 [details]
Patch

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

>> Source/WebCore/html/HTMLAnchorElement.cpp:125
>> +    return renderer && renderer->hasNonEmptyBox();
> 
> Unless this is going to have other clients I would just put it to HTMLAnchorElement. No need to bloat RenderBoxModelObject with questionable special case functions like this (any more than Node).

OK.
Comment 5 Darin Adler 2013-05-27 16:28:51 PDT 
Created attachment 203015 [details]
Patch
Comment 6 Antti Koivisto 2013-05-27 16:34:57 PDT 
Comment on attachment 203015 [details]
Patch

r=me
Comment 7 Early Warning System Bot 2013-05-27 16:36:57 PDT 
Comment on attachment 203015 [details]
Patch

Attachment 203015 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/683217
Comment 8 Early Warning System Bot 2013-05-27 16:40:44 PDT 
Comment on attachment 203015 [details]
Patch

Attachment 203015 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/683218
Comment 9 Build Bot 2013-05-27 16:51:41 PDT 
Comment on attachment 203015 [details]
Patch

Attachment 203015 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/666066
Comment 10 Darin Adler 2013-05-27 16:53:29 PDT 
Committed r150783: <http://trac.webkit.org/changeset/150783>
Comment 11 Lucas Forschler 2019-02-06 09:18:35 PST 
Mass move bugs into the DOM component.