Summary: | [Chromium] Add some notifications and an accessor to WebKit API | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jens Alfke <jens> | ||||||
Component: | WebKit API | Assignee: | Jens Alfke <jens> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, commit-queue, eric, levin, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Jens Alfke
2010-04-14 16:47:31 PDT
Created attachment 53447 [details]
Patch
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 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.
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.
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. Comment on attachment 54186 [details]
patch, after initial feedback
R=me
Comment on attachment 54186 [details] patch, after initial feedback Clearing flags on attachment: 54186 Committed r58362: <http://trac.webkit.org/changeset/58362> All reviewed patches have been landed. Closing bug. http://trac.webkit.org/changeset/58362 might have broken Qt Linux Release |