Chromium allows extensions to create popups that are sized to fit the popup's content. Browser action popups are an example. In order to compute the proper size, chromium needs to know the width and height scroll bars add to a window. Add a method to expose this information. This change will allow the following bug to be fixed: http://code.google.com/p/chromium/issues/detail?id=31494
Created attachment 51307 [details] Patch to add WebKit::WebFrameImpl::scrollbarsSize().
Attachment 51307 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebKit/chromium/ChangeLog:8: Line contains tab character. [whitespace/tab] [5] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 51311 [details] Style fix from previous patch.
Could you deduce this information from WebFrame::contentsSize() if you also had a WebFrame::visibleSize()? Maybe we should just add a WebFrame::visibleSize() method.
Comment on attachment 51311 [details] Style fix from previous patch. This really needs Darin Fisher's review since you are changing the public api, so these are just some drive by comments. > Index: WebKit/chromium/public/WebFrame.h > @@ -141,6 +141,11 @@ public: > // height required to display the document without scrollbars. > virtual int documentElementScrollHeight() const = 0; > > + // Return the size scrollbars add to the frame. This is the width of the One space before . in comments is the standard WebKit style. > + // horizontal scrollbar, and the height of the vertical scrollbar. A > + // nonexistent scrollbar has a width and height of zero. > + virtual WebSize scrollbarsSize() const = 0; > Index: WebKit/chromium/src/WebFrameImpl.cpp > +WebSize WebFrameImpl::scrollbarsSize() const > +{ ... > + if (m_frame) { > + WebCore::FrameView *view = m_frame->view(); Incorrect * placement. It should look like this: WebCore::FrameView* view = m_frame->view();
Comment on attachment 51311 [details] Style fix from previous patch. R- since there are open questions and i'd like to get this out of the review queue.
Sorry to not look at this for a while. Milestone-5 issues distracted me. (In reply to comment #4) > Could you deduce this information from WebFrame::contentsSize() if you also had > a WebFrame::visibleSize()? Maybe we should just add a WebFrame::visibleSize() > method. That could work if WebFrame::contentsSize() gave the size of the contents drawn on screen. However, it seems to give the size of the contents, including content you would need to scroll to see. I could add WebFrame::visibleSize() and WebFrame::visibleSizeIncludingScrollbars(), and subtract them. Or I could create WebFrame::visibleSize(bool includeScrollbarSize). Would one of those options be preferred to adding WebFrame:scrollbarsSize()? Sam
[Sorry for the delayed reply!] How about adding the following methods: int verticalScrollbarWidth() const; int horizontalScrollbarHeight() const; These can return 0 if the scrollbar does not exist.
Given that it's not clear how to fix the underlying bug here from the Chrome side, I'm closing this.