Bug 85661 - [BlackBerry] Add methods need by client side
Summary: [BlackBerry] Add methods need by client side
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit BlackBerry (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Crystal Zhang
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-04 13:46 PDT by Crystal Zhang
Modified: 2012-05-07 10:31 PDT (History)
4 users (show)

See Also:

Attachments
patch (1.59 KB, patch)
2012-05-04 13:46 PDT, Crystal Zhang 		no flags Details | Formatted Diff | Diff
updated patch (1.77 KB, patch)
2012-05-04 14:05 PDT, Crystal Zhang 		rwlbuis: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
fix commit error (1.77 KB, patch)
2012-05-04 14:29 PDT, Crystal Zhang 		rwlbuis: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
patch4 (1.65 KB, patch)
2012-05-04 15:45 PDT, Crystal Zhang 		tonikitoo: review-
tonikitoo: commit-queue-
Details | Formatted Diff | Diff
patch5 (1.79 KB, patch)
2012-05-07 07:43 PDT, Crystal Zhang 		tonikitoo: commit-queue-
Details | Formatted Diff | Diff
patch6 (1.81 KB, patch)
2012-05-07 08:19 PDT, Crystal Zhang 		no flags Details | Formatted Diff | Diff
Show Obsolete (5) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Crystal Zhang 2012-05-04 13:46:42 PDT 
Created attachment 140314 [details]
patch

The two APIs needed by client side.
Comment 1 WebKit Review Bot 2012-05-04 13:51:59 PDT 
Attachment 140314 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/blackberry/Api/WebPageClient..." exit_code: 1
Source/WebKit/blackberry/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Source/WebKit/blackberry/Api/WebPageClient.h:254:  The parameter name "rect" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 2 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 2 Crystal Zhang 2012-05-04 14:05:36 PDT 
Created attachment 140318 [details]
updated patch
Comment 3 Rob Buis 2012-05-04 14:16:40 PDT 
Comment on attachment 140318 [details]
updated patch

Looks good.
Comment 4 WebKit Review Bot 2012-05-04 14:21:13 PDT 
Comment on attachment 140318 [details]
updated patch

Rejecting attachment 140318 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

Last 500 characters of output:
ueue/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 70, in run_and_handle_errors
    self._run(tool, options, state)
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 64, in _run
    step(tool, options).run(state)
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/tool/steps/validatereviewer.py", line 50, in run
    if changelog_entry.has_valid_reviewer():
AttributeError: 'NoneType' object has no attribute 'has_valid_reviewer'

Full output: http://queues.webkit.org/results/12543443
Comment 5 Crystal Zhang 2012-05-04 14:29:20 PDT 
Created attachment 140329 [details]
fix commit error
Comment 6 Rob Buis 2012-05-04 14:33:07 PDT 
Comment on attachment 140329 [details]
fix commit error

Second attempt.
Comment 7 WebKit Review Bot 2012-05-04 14:41:27 PDT 
Comment on attachment 140329 [details]
fix commit error

Rejecting attachment 140329 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

Last 500 characters of output:
ueue/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 70, in run_and_handle_errors
    self._run(tool, options, state)
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 64, in _run
    step(tool, options).run(state)
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/tool/steps/validatereviewer.py", line 50, in run
    if changelog_entry.has_valid_reviewer():
AttributeError: 'NoneType' object has no attribute 'has_valid_reviewer'

Full output: http://queues.webkit.org/results/12543452
Comment 8 Crystal Zhang 2012-05-04 15:45:57 PDT 
Created attachment 140345 [details]
patch4
Comment 9 George Staikos 2012-05-04 17:33:20 PDT 
Comment on attachment 140345 [details]
patch4

Why is this not passing IntRect by const&?
Comment 10 Antonio Gomes 2012-05-04 19:12:48 PDT 
Comment on attachment 140345 [details]
patch4

View in context: https://bugs.webkit.org/attachment.cgi?id=140345&action=review

patches does not apply against ToT!

> Source/WebKit/blackberry/Api/WebPageClient.h:254
> +    virtual void createPopupWebView(Platform::IntRect) = 0;

const Platform::IntRect& and please name the parameter, if it has a meaningful name.
Comment 11 Antonio Gomes 2012-05-04 19:13:09 PDT 
> patches does not apply against ToT!

err, patch*
Comment 12 Crystal Zhang 2012-05-07 07:43:23 PDT 
Created attachment 140526 [details]
patch5
Comment 13 Antonio Gomes 2012-05-07 08:08:42 PDT 
Comment on attachment 140526 [details]
patch5

View in context: https://bugs.webkit.org/attachment.cgi?id=140526&action=review

> Source/WebKit/blackberry/Api/WebPageClient.h:255
> +    virtual void createPopupWebView(Platform::IntRect) = 0;
> +    virtual void closePopupWebView() = 0;

could you pass a 'const Platform::IntRect&' here, or you want a copy? Also please name the parameter as it is unclear what it is about.

> Source/WebKit/blackberry/ChangeLog:8
> +        Add methods need by client side when create and close HTML popup dialogs.

typo: needed
Comment 14 Crystal Zhang 2012-05-07 08:19:20 PDT 
Created attachment 140529 [details]
patch6
Comment 15 WebKit Review Bot 2012-05-07 10:31:42 PDT 
Comment on attachment 140529 [details]
patch6

Clearing flags on attachment: 140529

Committed r116318: <http://trac.webkit.org/changeset/116318>
Comment 16 WebKit Review Bot 2012-05-07 10:31:47 PDT 
All reviewed patches have been landed.  Closing bug.