WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
94086
[Chromium] Make the popup positioning code testable
https://bugs.webkit.org/show_bug.cgi?id=94086
Summary
[Chromium] Make the popup positioning code testable
Kent Tamura
Reported
2012-08-15 01:58:51 PDT
We have complex logic in PopupContainer::layoutAndCalculateWidgetRect(). But we don't have automated tests for it.
Attachments
Patch 1
(15.30 KB, patch)
2012-08-17 00:43 PDT
,
Kent Tamura
no flags
Details
Formatted Diff
Diff
Patch 2
(15.30 KB, patch)
2012-08-17 01:22 PDT
,
Kent Tamura
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Kent Tamura
Comment 1
2012-08-17 00:43:32 PDT
Created
attachment 159028
[details]
Patch 1
WebKit Review Bot
Comment 2
2012-08-17 01:14:08 PDT
Comment on
attachment 159028
[details]
Patch 1
Attachment 159028
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://queues.webkit.org/results/13518513
Kent Tamura
Comment 3
2012-08-17 01:22:07 PDT
Created
attachment 159043
[details]
Patch 2 Fix signed-unsinged comparison
Hajime Morrita
Comment 4
2012-08-20 18:41:26 PDT
Comment on
attachment 159043
[details]
Patch 2 View in context:
https://bugs.webkit.org/attachment.cgi?id=159043&action=review
> Source/WebCore/platform/chromium/PopupContainer.h:122 > + static IntRect layoutAndCalculateWidgetRectInternal(IntRect widgetRectInScreen, int targetControlHeight, const FloatRect& windowRect, const FloatRect& screen, bool isRTL, const int rtlOffset, PopupContent*, bool& needToResizeView);
This method has too many parameters. Probably these can be packed into a separate class then this method can be a part of that class.
Kent Tamura
Comment 5
2012-08-20 18:46:14 PDT
Comment on
attachment 159043
[details]
Patch 2 View in context:
https://bugs.webkit.org/attachment.cgi?id=159043&action=review
>> Source/WebCore/platform/chromium/PopupContainer.h:122 >> + static IntRect layoutAndCalculateWidgetRectInternal(IntRect widgetRectInScreen, int targetControlHeight, const FloatRect& windowRect, const FloatRect& screen, bool isRTL, const int rtlOffset, PopupContent*, bool& needToResizeView); > > This method has too many parameters. Probably these can be packed into a separate class then this method can be a part of that class.
Sounds good. I might work on it later.
WebKit Review Bot
Comment 6
2012-08-20 19:34:04 PDT
Comment on
attachment 159043
[details]
Patch 2 Clearing flags on attachment: 159043 Committed
r126121
: <
http://trac.webkit.org/changeset/126121
>
WebKit Review Bot
Comment 7
2012-08-20 19:34:08 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug