Bug 112278

Summary: [BlackBerry] Add Proximity Detector.
Product: WebKit Reporter: gmak
Component: WebKit BlackBerryAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: berto, gyuyoung.kim, mifenton, rakuco, rwlbuis, tonikitoo, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch with changelogs
none
Patch with changelogs - style fixed
rwlbuis: review-
Patch with changelogs
none
Patch with changelogs
none
Patch none

Description gmak 2013-03-13 12:47:09 PDT
It detects blocks near the given point and returns the top left corner of the selected rect.
Comment 1 gmak 2013-03-13 12:53:42 PDT
Created attachment 192969 [details]
Patch with changelogs
Comment 2 WebKit Review Bot 2013-03-13 12:56:07 PDT
Attachment 192969 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/ChangeLog', u'Source/WebKit/PlatformBlackBerry.cmake', u'Source/WebKit/blackberry/ChangeLog', u'Source/WebKit/blackberry/WebKitSupport/ProximityDetector.cpp', u'Source/WebKit/blackberry/WebKitSupport/ProximityDetector.h']" exit_code: 1
Source/WebKit/blackberry/WebKitSupport/ProximityDetector.cpp:103:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit/blackberry/WebKitSupport/ProximityDetector.cpp:104:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit/blackberry/WebKitSupport/ProximityDetector.cpp:105:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit/blackberry/WebKitSupport/ProximityDetector.cpp:107:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit/blackberry/WebKitSupport/ProximityDetector.cpp:108:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit/blackberry/WebKitSupport/ProximityDetector.cpp:109:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit/blackberry/WebKitSupport/ProximityDetector.cpp:110:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit/blackberry/WebKitSupport/ProximityDetector.cpp:111:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 8 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 gmak 2013-03-13 13:01:44 PDT
Created attachment 192975 [details]
Patch with changelogs - style fixed
Comment 4 WebKit Review Bot 2013-03-13 13:09:45 PDT
Attachment 192975 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/ChangeLog', u'Source/WebKit/PlatformBlackBerry.cmake', u'Source/WebKit/blackberry/ChangeLog', u'Source/WebKit/blackberry/WebKitSupport/ProximityDetector.cpp', u'Source/WebKit/blackberry/WebKitSupport/ProximityDetector.h']" exit_code: 1
Source/WebKit/blackberry/WebKitSupport/ProximityDetector.cpp:108:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Rob Buis 2013-03-13 13:11:59 PDT
Comment on attachment 192975 [details]
Patch with changelogs - style fixed

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

Can be cleaned up a bit more.

> Source/WebKit/blackberry/WebKitSupport/ProximityDetector.cpp:18
> +using WTF::RefPtr;

Should not be needed at all.

> Source/WebKit/blackberry/WebKitSupport/ProximityDetector.cpp:19
> +using WTF::String;

You may get away with discarding this one but it depends if it clashes with B::P::String.
Comment 6 gmak 2013-03-13 13:23:02 PDT
Created attachment 192983 [details]
Patch with changelogs
Comment 7 Rob Buis 2013-03-13 13:34:59 PDT
Comment on attachment 192983 [details]
Patch with changelogs 

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

> Source/WebKit/blackberry/WebKitSupport/ProximityDetector.cpp:104
> +        bool equalPriorityAndCloser = priority == bestPriority && sqrt((double)contentPos.distanceSquaredToPoint(bestPoint)) > sqrt((double)contentPos.distanceSquaredToPoint(curRect.location()))

I think parentheses would make this more readable, plus you can discard both sqrt in this case.
Comment 8 gmak 2013-03-13 14:06:28 PDT
Created attachment 192991 [details]
Patch with changelogs
Comment 9 Rob Buis 2013-03-13 14:11:03 PDT
Comment on attachment 192991 [details]
Patch with changelogs

LGTM.
Comment 10 WebKit Review Bot 2013-03-13 14:51:47 PDT
Comment on attachment 192991 [details]
Patch with changelogs

Clearing flags on attachment: 192991

Committed r145757: <http://trac.webkit.org/changeset/145757>
Comment 11 WebKit Review Bot 2013-03-13 14:51:52 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Alberto Garcia 2013-03-14 05:29:18 PDT
Created attachment 193107 [details]
Patch

This will not compile, variable names are wrong. Here's the fix.
Comment 13 Rob Buis 2013-03-14 07:32:30 PDT
Comment on attachment 193107 [details]
Patch

Again, well spotted.
Comment 14 Alberto Garcia 2013-03-18 10:49:56 PDT
I think the patch was not commited, either we reopen this or I file a new bug.
Comment 15 Rob Buis 2013-03-18 11:09:19 PDT
Reopening to try to commit a fixup patch.
Comment 16 Rob Buis 2013-03-18 11:10:25 PDT
Comment on attachment 193107 [details]
Patch

Retrying.
Comment 17 WebKit Review Bot 2013-03-18 11:32:03 PDT
Comment on attachment 193107 [details]
Patch

Clearing flags on attachment: 193107

Committed r146090: <http://trac.webkit.org/changeset/146090>
Comment 18 WebKit Review Bot 2013-03-18 11:32:08 PDT
All reviewed patches have been landed.  Closing bug.
Comment 19 gmak 2013-03-18 12:14:30 PDT
Thanks for catching that, I never compiled it because we have an older non-webkit compliant version internally.