Bug 137416 - Move ServicesOverlayController to WebCore
Summary: Move ServicesOverlayController to WebCore
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-10-03 19:56 PDT by Tim Horton
Modified: 2014-10-09 13:53 PDT (History)
8 users (show)

See Also:

Attachments
wip (101.93 KB, patch)
2014-10-03 19:57 PDT, Tim Horton 		no flags Details | Formatted Diff | Diff
patch (107.62 KB, patch)
2014-10-06 14:56 PDT, Tim Horton 		no flags Details | Formatted Diff | Diff
move to mainframe (109.47 KB, patch)
2014-10-09 12:28 PDT, Tim Horton 		no flags Details | Formatted Diff | Diff
fix mac build (109.47 KB, patch)
2014-10-09 13:02 PDT, Tim Horton 		andersca: review+
Details | Formatted Diff | Diff
Show Obsolete (3) 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 Tim Horton 2014-10-03 19:56:46 PDT 
The platform dependent bits are fairly small; this is a step on the way to having ServicesOverlayController work for Legacy WebKit.
Comment 1 Radar WebKit Bug Importer 2014-10-03 19:56:53 PDT 
<rdar://problem/18546283>
Comment 2 Tim Horton 2014-10-03 19:57:00 PDT 
Created attachment 239259 [details]
wip
Comment 3 WebKit Commit Bot 2014-10-03 19:58:24 PDT 
Attachment 239259 [details] did not pass style-queue:


ERROR: Source/WebCore/editing/SelectionRectGatherer.cpp:33:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/page/mac/ServicesOverlayController.mm:186:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/page/mac/ServicesOverlayController.mm:392:  Should have a space between // and comment  [whitespace/comments] [4]
ERROR: Source/WebCore/page/mac/ServicesOverlayController.mm:405:  Should have a space between // and comment  [whitespace/comments] [4]
Total errors found: 4 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Tim Horton 2014-10-06 14:56:22 PDT 
Created attachment 239360 [details]
patch
Comment 5 WebKit Commit Bot 2014-10-06 14:59:20 PDT 
Attachment 239360 [details] did not pass style-queue:


ERROR: Source/WebCore/page/mac/ServicesOverlayController.mm:185:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 1 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Anders Carlsson 2014-10-09 11:36:52 PDT 
Comment on attachment 239360 [details]
patch

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

I think this should go on MainFrame instead of Page.

> Source/WebCore/page/mac/ServicesOverlayController.h:48
> +    ServicesOverlayController(Page&);

I think this should be explicit.
Comment 7 Tim Horton 2014-10-09 12:28:24 PDT 
Created attachment 239555 [details]
move to mainframe
Comment 8 WebKit Commit Bot 2014-10-09 12:31:06 PDT 
Attachment 239555 [details] did not pass style-queue:


ERROR: Source/WebCore/page/mac/ServicesOverlayController.mm:194:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 1 in 18 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Tim Horton 2014-10-09 13:02:06 PDT 
Created attachment 239562 [details]
fix mac build
Comment 10 WebKit Commit Bot 2014-10-09 13:04:56 PDT 
Attachment 239562 [details] did not pass style-queue:


ERROR: Source/WebCore/page/mac/ServicesOverlayController.mm:194:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 1 in 18 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 Anders Carlsson 2014-10-09 13:22:20 PDT 
Comment on attachment 239562 [details]
fix mac build

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

> Source/WebCore/editing/Editor.cpp:3347
> +    if (!shouldDetectTelephoneNumbers() || !client() || !m_frame.page())

I don't think you need this.
Comment 12 Tim Horton 2014-10-09 13:53:31 PDT 
http://trac.webkit.org/changeset/174524