WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
181110
Fix build failures due to using deprecated AppKit symbols
https://bugs.webkit.org/show_bug.cgi?id=181110
Summary
Fix build failures due to using deprecated AppKit symbols
Wenson Hsieh
Reported
2017-12-21 17:05:48 PST
<
rdar://problem/36162865
>
Attachments
Test pass for EWS
(122.53 KB, patch)
2017-12-21 17:21 PST
,
Wenson Hsieh
no flags
Details
Formatted Diff
Diff
Try to fix the build for 10.11
(130.69 KB, patch)
2017-12-21 17:57 PST
,
Wenson Hsieh
no flags
Details
Formatted Diff
Diff
More SDK compat fixes.
(132.76 KB, patch)
2017-12-21 19:35 PST
,
Wenson Hsieh
no flags
Details
Formatted Diff
Diff
Try to get El Capitan to build (3)
(133.39 KB, patch)
2017-12-21 20:14 PST
,
Wenson Hsieh
no flags
Details
Formatted Diff
Diff
Try to get El Capitan to build (4)
(133.90 KB, patch)
2017-12-21 20:36 PST
,
Wenson Hsieh
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews101 for mac-elcapitan
(2.25 MB, application/zip)
2017-12-21 21:24 PST
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews117 for mac-elcapitan
(3.02 MB, application/zip)
2017-12-21 22:09 PST
,
EWS Watchlist
no flags
Details
Fix MiniBrowser
(151.96 KB, patch)
2017-12-22 00:12 PST
,
Wenson Hsieh
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews106 for mac-elcapitan-wk2
(2.57 MB, application/zip)
2017-12-22 01:25 PST
,
EWS Watchlist
no flags
Details
Keep using legacy pasteboard constants.
(127.62 KB, patch)
2017-12-22 04:08 PST
,
Wenson Hsieh
no flags
Details
Formatted Diff
Diff
Don't spam deprecation guards.
(171.09 KB, patch)
2017-12-22 06:21 PST
,
Wenson Hsieh
mitz: review+
Details
Formatted Diff
Diff
Patch for landing
(169.37 KB, patch)
2017-12-22 13:09 PST
,
Wenson Hsieh
commit-queue
: commit-queue-
Details
Formatted Diff
Diff
Hold for EWS
(165.33 KB, patch)
2017-12-22 13:18 PST
,
Wenson Hsieh
no flags
Details
Formatted Diff
Diff
Show Obsolete
(10)
View All
Add attachment
proposed patch, testcase, etc.
Wenson Hsieh
Comment 1
2017-12-21 17:21:10 PST
Created
attachment 330081
[details]
Test pass for EWS
EWS Watchlist
Comment 2
2017-12-21 17:25:15 PST
Attachment 330081
[details]
did not pass style-queue: ERROR: Source/WebKitLegacy/mac/WebView/WebHTMLView.mm:1167: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4] ERROR: Source/WebKitLegacy/mac/WebView/WebHTMLView.mm:1167: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/WebKitLegacy/mac/WebView/WebHTMLView.mm:1169: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebKitLegacy/mac/WebView/WebHTMLView.mm:1174: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4] ERROR: Source/WebKitLegacy/mac/WebView/WebHTMLView.mm:1174: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/WebKitLegacy/mac/WebView/WebHTMLView.mm:1176: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebKitLegacy/mac/WebView/WebHTMLView.mm:1181: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4] ERROR: Source/WebKitLegacy/mac/WebView/WebHTMLView.mm:1181: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/WebKitLegacy/mac/WebView/WebHTMLView.mm:1183: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebKitLegacy/mac/WebView/WebHTMLView.mm:1188: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4] ERROR: Source/WebKitLegacy/mac/WebView/WebHTMLView.mm:1188: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/WebKitLegacy/mac/WebView/WebHTMLView.mm:1190: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebKitLegacy/mac/WebView/WebHTMLView.mm:1195: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4] ERROR: Source/WebKitLegacy/mac/WebView/WebHTMLView.mm:1195: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/WebKitLegacy/mac/WebView/WebHTMLView.mm:1197: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebKitLegacy/mac/WebView/WebHTMLView.mm:1209: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4] ERROR: Source/WebKitLegacy/mac/WebView/WebHTMLView.mm:1209: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/WebKitLegacy/mac/WebView/WebHTMLView.mm:1211: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebKitLegacy/mac/WebView/WebHTMLView.mm:1216: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4] ERROR: Source/WebKitLegacy/mac/WebView/WebHTMLView.mm:1216: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/WebKitLegacy/mac/WebView/WebHTMLView.mm:1218: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/mac/PasteboardWriter.mm:81: Extra space in capture list. [whitespace/brackets] [4] ERROR: Source/WebCore/platform/mac/PasteboardWriter.mm:83: Extra space in capture list. [whitespace/brackets] [4] ERROR: Source/WebCore/platform/mac/PasteboardWriter.mm:85: Extra space in capture list. [whitespace/brackets] [4] Total errors found: 24 in 39 files If any of these errors are false positives, please file a bug against check-webkit-style.
Wenson Hsieh
Comment 3
2017-12-21 17:57:39 PST
Created
attachment 330086
[details]
Try to fix the build for 10.11
Tim Horton
Comment 4
2017-12-21 18:11:12 PST
Comment on
attachment 330086
[details]
Try to fix the build for 10.11 View in context:
https://bugs.webkit.org/attachment.cgi?id=330086&action=review
rs=me once you get the build green
> Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm:860 > +#pragma GCC diagnostic push
why GCC instead of clang :P
Wenson Hsieh
Comment 5
2017-12-21 19:15:36 PST
(In reply to Tim Horton from
comment #4
)
> Comment on
attachment 330086
[details]
> Try to fix the build for 10.11 > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=330086&action=review
> > rs=me once you get the build green
Thanks! Though, I might need to make a few slightly more aggressive tweaks to get everything green...it would be nice to get a second opinion :P
> > > Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm:860 > > +#pragma GCC diagnostic push > > why GCC instead of clang :P
‼️ Good catch, fixed
Wenson Hsieh
Comment 6
2017-12-21 19:35:22 PST
Created
attachment 330094
[details]
More SDK compat fixes.
mitz
Comment 7
2017-12-21 19:39:35 PST
I guess I don’t understand how you can not handle the deprecated pasteboard types—couldn’t clients still be vending or expecting them, especially clients built with older SDKs? Is something else along the way (AppKit?) doing the mapping between the types?
Wenson Hsieh
Comment 8
2017-12-21 20:14:12 PST
Created
attachment 330097
[details]
Try to get El Capitan to build (3)
Wenson Hsieh
Comment 9
2017-12-21 20:21:16 PST
(In reply to mitz from
comment #7
)
> I guess I don’t understand how you can not handle the deprecated pasteboard > types—couldn’t clients still be vending or expecting them, especially > clients built with older SDKs? Is something else along the way (AppKit?) > doing the mapping between the types?
I reached out to the AppKit folks who made the change, and confirmed that AppKit maps between between modern (UTI) and legacy pasteboard types. I also specifically pointed out the scenario in which a client iterates over the list of registered pasteboard types in search of a deprecated pasteboard type string, and it appears that case is handled as well.
Wenson Hsieh
Comment 10
2017-12-21 20:36:49 PST
Created
attachment 330101
[details]
Try to get El Capitan to build (4)
EWS Watchlist
Comment 11
2017-12-21 21:24:53 PST
Comment on
attachment 330101
[details]
Try to get El Capitan to build (4)
Attachment 330101
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.webkit.org/results/5798059
New failing tests: fast/events/ondrop-text-html.html editing/pasteboard/drag-drop-input-textarea.html fast/events/input-events-insert-by-drop.html editing/pasteboard/get-data-text-plain-drop.html editing/pasteboard/drop-text-events.html fast/forms/drag-out-of-textarea.html fast/events/input-events-drag-and-drop.html fast/forms/drag-into-textarea.html editing/pasteboard/drop-inputtext-acquires-style.html fast/events/5056619.html editing/pasteboard/data-transfer-get-data-on-drop-rich-text.html fast/events/dropzone-003.html editing/pasteboard/drag-drop-url-text.html editing/pasteboard/data-transfer-get-data-on-drop-plain-text.html editing/pasteboard/data-transfer-get-data-on-drop-url.html
EWS Watchlist
Comment 12
2017-12-21 21:24:54 PST
Created
attachment 330108
[details]
Archive of layout-test-results from ews101 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Wenson Hsieh
Comment 13
2017-12-21 21:42:27 PST
(In reply to Build Bot from
comment #11
)
> Comment on
attachment 330101
[details]
> Try to get El Capitan to build (4) > >
Attachment 330101
[details]
did not pass mac-ews (mac): > Output:
http://webkit-queues.webkit.org/results/5798059
> > New failing tests: > fast/events/ondrop-text-html.html > editing/pasteboard/drag-drop-input-textarea.html > fast/events/input-events-insert-by-drop.html > editing/pasteboard/get-data-text-plain-drop.html > editing/pasteboard/drop-text-events.html > fast/forms/drag-out-of-textarea.html > fast/events/input-events-drag-and-drop.html > fast/forms/drag-into-textarea.html > editing/pasteboard/drop-inputtext-acquires-style.html > fast/events/5056619.html > editing/pasteboard/data-transfer-get-data-on-drop-rich-text.html > fast/events/dropzone-003.html > editing/pasteboard/drag-drop-url-text.html > editing/pasteboard/data-transfer-get-data-on-drop-plain-text.html > editing/pasteboard/data-transfer-get-data-on-drop-url.html
Curiously, these are all drag and drop related; copy and paste tests look OK. I'm also not seeing these test failures when running against the latest SDK. I tried manually running some of these test cases in Safari, and it doesn't seem broken. This makes me a little suspicious of DumpRenderTreePasteboard. Perhaps behavior there doesn't match up with the platform after this change?
EWS Watchlist
Comment 14
2017-12-21 22:09:41 PST
Comment on
attachment 330101
[details]
Try to get El Capitan to build (4)
Attachment 330101
[details]
did not pass mac-debug-ews (mac): Output:
http://webkit-queues.webkit.org/results/5798385
New failing tests: fast/events/ondrop-text-html.html editing/pasteboard/drag-drop-input-textarea.html fast/events/input-events-insert-by-drop.html editing/pasteboard/get-data-text-plain-drop.html editing/pasteboard/drop-text-events.html fast/forms/drag-out-of-textarea.html fast/events/input-events-drag-and-drop.html fast/forms/drag-into-textarea.html editing/pasteboard/drop-inputtext-acquires-style.html fast/events/5056619.html editing/pasteboard/data-transfer-get-data-on-drop-rich-text.html fast/events/dropzone-003.html editing/pasteboard/drag-drop-url-text.html editing/pasteboard/data-transfer-get-data-on-drop-plain-text.html editing/pasteboard/data-transfer-get-data-on-drop-url.html
EWS Watchlist
Comment 15
2017-12-21 22:09:42 PST
Created
attachment 330110
[details]
Archive of layout-test-results from ews117 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Alexey Proskuryakov
Comment 16
2017-12-21 23:53:50 PST
> I tried manually running some of these test cases in Safari, and it doesn't seem broken.
Were you testing El Capitan, like EWS does?
Wenson Hsieh
Comment 17
2017-12-22 00:12:57 PST
Created
attachment 330111
[details]
Fix MiniBrowser
Wenson Hsieh
Comment 18
2017-12-22 00:19:44 PST
(In reply to Alexey Proskuryakov from
comment #16
)
> > I tried manually running some of these test cases in Safari, and it doesn't seem broken. > > Were you testing El Capitan, like EWS does?
I was not. After building/testing on El Capitan, I realized that most of the modern pasteboard types (except URL) are present on 10.11, so AppKitCompatibilityDeclarations.h should not be trying to redefine them to be the deprecated versions. Fixing this fixed the test failures for me.
EWS Watchlist
Comment 19
2017-12-22 01:25:52 PST
Comment on
attachment 330111
[details]
Fix MiniBrowser
Attachment 330111
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.webkit.org/results/5799523
New failing tests: editing/pasteboard/data-transfer-get-data-on-paste-as-plain-text-when-custom-pasteboard-data-disabled.html
EWS Watchlist
Comment 20
2017-12-22 01:25:53 PST
Created
attachment 330113
[details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Wenson Hsieh
Comment 21
2017-12-22 02:56:16 PST
(In reply to Build Bot from
comment #19
)
> Comment on
attachment 330111
[details]
> Fix MiniBrowser > >
Attachment 330111
[details]
did not pass mac-wk2-ews (mac-wk2): > Output:
http://webkit-queues.webkit.org/results/5799523
> > New failing tests: > editing/pasteboard/data-transfer-get-data-on-paste-as-plain-text-when-custom- > pasteboard-data-disabled.html
This failure is harder to explain. Logging in this test shows that this fails because Pasteboard::addHTMLClipboardTypesForCocoaType uses utiTypeFromCocoaType to transform "Cocoa" types into either UTIs or MIME types (which, as an aside, would probably be a more accurate name for this function). Before this change, NSHTMLPboardType is passed in, which UTTypeCreatePreferredIdentifierForTag understands. After this change, "public.html" is given, so our logic to convert to a MIME type fails. This case in particular is trivial to address by teaching utiTypeFromCocoaType to go directly from UTI to MIME, rather than go from legacy Pboard type to UTI to MIME. However, this makes me wonder why -[NSPasteboard types] didn't return a list of types containing both legacy Pboard and modern UTI-based types, like it does in a simple test app. Without understanding why the list of pasteboard types isn't being duplicated in this scenario, I'm afraid compatibility might be a bigger risk than previously thought. I'm going to need more time to investigate this, and will also probably need to poke around in AppKit to understand their code for duplicating pasteboard types. In the interest of fixing the build (and mitigating risk), we should probably resort to just suppressing pasteboard deprecation warnings...
Wenson Hsieh
Comment 22
2017-12-22 04:08:54 PST
Created
attachment 330117
[details]
Keep using legacy pasteboard constants.
Wenson Hsieh
Comment 23
2017-12-22 06:21:36 PST
Created
attachment 330123
[details]
Don't spam deprecation guards.
mitz
Comment 24
2017-12-22 08:23:08 PST
Comment on
attachment 330123
[details]
Don't spam deprecation guards. View in context:
https://bugs.webkit.org/attachment.cgi?id=330123&action=review
> Source/WebCore/platform/mac/PasteboardMac.mm:-208 > -
Why did we lose this newline?
> Source/WebCore/platform/mac/PasteboardMac.mm:664 > +
Oh, it went here :p
> Source/WebCore/platform/mac/PasteboardWriter.mm:80 > + // NSURLPboardType().
?
> Source/WebCore/platform/mac/PasteboardWriter.mm:82 > + [pasteboardItem setPropertyList:@[cocoaURL.relativeString, baseCocoaURL.absoluteString] forType:toUTI(WebCore::legacyURLPasteboardType()).get()];
Pretty sure the prevailing style is with spaces, similarly to C struct and array initializers.
> Source/WebKitLegacy/mac/WebView/WebView.h:173 > + including legacyURLPasteboardType() to find a URL on the pasteboard.
The comments in the API header shouldn’t refer to this WebCore-internal concept!
Wenson Hsieh
Comment 25
2017-12-22 12:51:26 PST
Comment on
attachment 330123
[details]
Don't spam deprecation guards. View in context:
https://bugs.webkit.org/attachment.cgi?id=330123&action=review
Thanks for the review!
>> Source/WebCore/platform/mac/PasteboardMac.mm:-208 >> - > > Why did we lose this newline?
Whoops, that wasn't intended. Fixed!
>> Source/WebCore/platform/mac/PasteboardMac.mm:664 >> + > > Oh, it went here :p
:P Removed.
>> Source/WebCore/platform/mac/PasteboardWriter.mm:80 >> + // NSURLPboardType(). > > ?
Ah, find-and-replace error. Fixed!
>> Source/WebCore/platform/mac/PasteboardWriter.mm:82 >> + [pasteboardItem setPropertyList:@[cocoaURL.relativeString, baseCocoaURL.absoluteString] forType:toUTI(WebCore::legacyURLPasteboardType()).get()]; > > Pretty sure the prevailing style is with spaces, similarly to C struct and array initializers.
Oh, ok. Restored spaces in @[ ] I had removed these because check-webkit-style complained here with: ERROR: Source/WebCore/platform/mac/PasteboardWriter.mm:82: Extra space in capture list. [whitespace/brackets] [4] ERROR: Source/WebCore/platform/mac/PasteboardWriter.mm:84: Extra space in capture list. [whitespace/brackets] [4] ERROR: Source/WebCore/platform/mac/PasteboardWriter.mm:86: Extra space in capture list. [whitespace/brackets] [4]
>> Source/WebKitLegacy/mac/WebView/WebView.h:173 >> + including legacyURLPasteboardType() to find a URL on the pasteboard. > > The comments in the API header shouldn’t refer to this WebCore-internal concept!
Oops, another unintended find and replace! Fixed.
Wenson Hsieh
Comment 26
2017-12-22 13:09:51 PST
Created
attachment 330141
[details]
Patch for landing
WebKit Commit Bot
Comment 27
2017-12-22 13:11:43 PST
Comment on
attachment 330141
[details]
Patch for landing Rejecting
attachment 330141
[details]
from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'apply-attachment', '--no-update', '--non-interactive', 330141, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: ac/WebView/WebView.mm patching file Tools/ChangeLog patching file Tools/MiniBrowser/AppKitCompatibilityDeclarations.h patching file Tools/MiniBrowser/MiniBrowser.xcodeproj/project.pbxproj patching file Tools/MiniBrowser/mac/SettingsController.m patching file Tools/MiniBrowser/mac/WK1BrowserWindowController.m patching file Tools/MiniBrowser/mac/WK2BrowserWindowController.m Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Full output:
http://webkit-queues.webkit.org/results/5804798
Wenson Hsieh
Comment 28
2017-12-22 13:18:11 PST
Created
attachment 330142
[details]
Hold for EWS
EWS Watchlist
Comment 29
2017-12-22 13:20:52 PST
Attachment 330142
[details]
did not pass style-queue: ERROR: Source/WebCore/platform/mac/PasteboardWriter.mm:82: Extra space in capture list. [whitespace/brackets] [4] ERROR: Source/WebCore/platform/mac/PasteboardWriter.mm:84: Extra space in capture list. [whitespace/brackets] [4] ERROR: Source/WebCore/platform/mac/PasteboardWriter.mm:86: Extra space in capture list. [whitespace/brackets] [4] Total errors found: 3 in 47 files If any of these errors are false positives, please file a bug against check-webkit-style.
WebKit Commit Bot
Comment 30
2017-12-22 14:23:32 PST
Comment on
attachment 330142
[details]
Hold for EWS Clearing flags on attachment: 330142 Committed
r226277
: <
https://trac.webkit.org/changeset/226277
>
Wenson Hsieh
Comment 31
2018-01-04 15:26:26 PST
These deprecation warnings have been addressed. See:
r226277
,
r226279
,
r226281
,
r226284
,
r226286
.
Radar WebKit Bug Importer
Comment 32
2018-01-04 15:27:27 PST
<
rdar://problem/36307587
>
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