Bug 37625 – [Chromium] Add some notifications and an accessor to WebKit API

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit API

[Jens Alfke]

For some work I'm doing on Chromium* I need to add a few methods to the WebKit API to expose WebCore functionality:

1. WebElement::isVisible — tells whether a DOM element is visible (defined as 'has a layout with a non-empty bounding box').
2. WebFrame::didFirstLayout and WebFrame::didFirstVisuallyNonEmptyLayout — these are equivalent to the FrameLoaderClient methods of the same names.

* http://code.google.com/p/chromium/issues/detail?id=777

[Jens Alfke]

Created attachment 53447 [details]
Patch

[Jens Alfke]

I actually linked to the wrong Chromium bug, below. The correct one is
"Password manager heuristics fail for sites that keep the login form around after signin"
http://code.google.com/p/chromium/issues/detail?id=28911

[Darin Fisher (:fishd, Google)]

Comment on attachment 53447 [details]
Patch

WebKit/chromium/src/WebElement.cpp:76
 +  // Note: This method only works properly after layout has occurred.
This comment would be helpful in the public header file.

WebKit/chromium/src/WebElement.cpp:79
 +      // (Copied from HTMLAnchorElement::isKeyboardFocusable)
It seems rather unfortunate to duplicate this code.  Can it be shared somehow?

WebKit/chromium/public/WebFrameClient.h:198
 +      // The frame's document finished the initial layout of a page.
This would probably be better in the "Geometry notifications" section instead of the "Navigational notifications" section.

WebKit/chromium/public/WebElement.h:60
 +          WEBKIT_API bool isVisible() const;
how about naming this method hasNonEmptyBoundingBox?  that way the method name is self documenting and more accurate.  if a consumer wants to use that to test visibility, then the fact that it is not a 100% guarantee is an issue for the user instead of a problem with the implementation of this method.

[Jens Alfke]

Created attachment 54186 [details]
patch, after initial feedback

OK, I have moved the hasNonEmptyBoundingBox method up to Node, and made the other changes you suggested.

[Jens Alfke]

The Chromium patch (fix for 28911) that's depending on this is now approved, and waiting on this patch to get checked in and downstreamed to Chromium first.

[Darin Fisher (:fishd, Google)]

Comment on attachment 54186 [details]
patch, after initial feedback

R=me

[WebKit Commit Bot]

Comment on attachment 54186 [details]
patch, after initial feedback

Clearing flags on attachment: 54186

Committed r58362: <http://trac.webkit.org/changeset/58362>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/58362 might have broken Qt Linux Release