Bug 73632

Summary: Upstream 7 files into WebCore/platform/blackberry
Product: WebKit Reporter: Mary Wu <mawu>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: dbates, dglazkov, tonikitoo, webkit.review.bot, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 73144    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Mary Wu
Reported 2011-12-01 23:58:46 PST
Upstream following 7 files into WebCore/platform/blackberry: PopupMenuBlackBerry.cpp ScrollbarBlackBerry.cpp ScrollViewBlackBerry.cpp SearchPopupMenuBlackBerry.h PopupMenuBlackBerry.h ScrollbarThemeBlackBerry.cpp SearchPopupMenuBlackBerry.cpp
Attachments
Patch (13.04 KB, patch)
2011-12-02 00:02 PST, Mary Wu 		no flags
DetailsFormatted DiffDiff
Patch (12.87 KB, patch)
2011-12-04 18:42 PST, Mary Wu 		no flags
DetailsFormatted DiffDiff
Patch (11.07 KB, patch)
2011-12-05 01:08 PST, Mary Wu 		no flags
DetailsFormatted DiffDiff
Patch (11.12 KB, patch)
2011-12-05 19:27 PST, Mary Wu 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Mary Wu
Comment 1 2011-12-02 00:02:01 PST
Created attachment 117582 [details] Patch
Eric Seidel (no email)
Comment 2 2011-12-03 14:20:50 PST
Comment on attachment 117582 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=117582&action=review > Source/WebCore/platform/blackberry/PopupMenuBlackBerry.cpp:31 > + // no-op These comments don't add anythign. > Source/WebCore/platform/blackberry/SearchPopupMenuBlackBerry.cpp:52 > + // FIXME: remove 0 template parameter when this is implemented Why is the 0 template parameter there in the first place?
Mary Wu
Comment 3 2011-12-04 18:42:00 PST
(In reply to comment #2) > (From update of attachment 117582 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=117582&action=review > > > Source/WebCore/platform/blackberry/PopupMenuBlackBerry.cpp:31 > > + // no-op > > These comments don't add anythign. > > > Source/WebCore/platform/blackberry/SearchPopupMenuBlackBerry.cpp:52 > > + // FIXME: remove 0 template parameter when this is implemented > > Why is the 0 template parameter there in the first place? ok, thanks. Will update...
Mary Wu
Comment 4 2011-12-04 18:42:43 PST
Created attachment 117822 [details] Patch
WebKit Review Bot
Comment 5 2011-12-04 19:31:43 PST
Comment on attachment 117822 [details] Patch Attachment 117822 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10734542 New failing tests: svg/custom/linking-uri-01-b.svg
Mary Wu
Comment 6 2011-12-04 21:26:59 PST
(In reply to comment #5) > (From update of attachment 117822 [details]) > Attachment 117822 [details] did not pass chromium-ews (chromium-xvfb): > Output: http://queues.webkit.org/results/10734542 > > New failing tests: > svg/custom/linking-uri-01-b.svg it's blackberry specific porting, shouldn't impact chrome test/build
Daniel Bates
Comment 7 2011-12-05 00:18:37 PST
Comment on attachment 117822 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=117822&action=review > Source/WebCore/platform/blackberry/PopupMenuBlackBerry.cpp:33 > +void PopupMenuBlackBerry::show(const IntRect& r, FrameView* v, int index) All of these parameters are unused. Either we need to use UNUSED_PARAM() or we need to omit them from the signature here or this may cause unused variable warnings depending on the compiler warning level. > Source/WebCore/platform/blackberry/ScrollViewBlackBerry.cpp:22 > +namespace WebCore { Why do we need to commit this almost empty file? > Source/WebCore/platform/blackberry/ScrollbarBlackBerry.cpp:22 > +namespace WebCore { Ditto.
Mary Wu
Comment 8 2011-12-05 01:08:02 PST
Created attachment 117854 [details] Patch
WebKit Review Bot
Comment 9 2011-12-05 08:46:00 PST
Comment on attachment 117854 [details] Patch Attachment 117854 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10703592 New failing tests: svg/custom/linking-uri-01-b.svg
Rob Buis
Comment 10 2011-12-05 09:11:36 PST
Comment on attachment 117854 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=117854&action=review Looks good, nearly there, could be improved a bit, so r-. > Source/WebCore/platform/blackberry/PopupMenuBlackBerry.h:23 > +#include "PopupMenuClient.h" I think a forward reference for PopupMenuClient should do. > Source/WebCore/platform/blackberry/ScrollbarThemeBlackBerry.cpp:29 > + static ScrollbarTheme theme; Can DEFINE_STATIC_LOCAL be used here?
Mary Wu
Comment 11 2011-12-05 19:27:28 PST
Created attachment 117979 [details] Patch
Rob Buis
Comment 12 2011-12-07 08:49:34 PST
Comment on attachment 117979 [details] Patch LGTM
WebKit Review Bot
Comment 13 2011-12-07 10:24:53 PST
Comment on attachment 117979 [details] Patch Clearing flags on attachment: 117979 Committed r102249: <http://trac.webkit.org/changeset/102249>
WebKit Review Bot
Comment 14 2011-12-07 10:24:58 PST
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.