Bug 124178 - Start moving off of NSCalendarDate
Summary: Start moving off of NSCalendarDate
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-11-11 17:42 PST by Ricky Mondello
Modified: 2013-11-18 21:12 PST (History)
6 users (show)

See Also:


Attachments
First attempt to run through EWS. (8.91 KB, patch)
2013-11-11 17:46 PST, Ricky Mondello
buildbot: commit-queue-
Details | Formatted Diff | Diff
Another attempt; might work for 10.8 (8.97 KB, patch)
2013-11-11 18:09 PST, Ricky Mondello
no flags Details | Formatted Diff | Diff
This time, don’t modify API. (10.09 KB, patch)
2013-11-11 23:42 PST, Ricky Mondello
andersca: review-
Details | Formatted Diff | Diff
Use NS_DEPRECATED_MAC instead of #pragma-ing API and code supporting API (10.26 KB, patch)
2013-11-12 12:50 PST, Ricky Mondello
buildbot: commit-queue-
Details | Formatted Diff | Diff
Disable the deprecation warnings (5.09 KB, patch)
2013-11-18 20:36 PST, Mark Rowe (bdash)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ricky Mondello 2013-11-11 17:42:33 PST
NSCalendarDate has been deprecated as of 10.6. WebKit should stop using it in favor of NSDate and NSDateComponents.
Comment 1 Ricky Mondello 2013-11-11 17:46:56 PST
Created attachment 216627 [details]
First attempt to run through EWS.
Comment 2 Build Bot 2013-11-11 17:50:34 PST
Comment on attachment 216627 [details]
First attempt to run through EWS.

Attachment 216627 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/22589461
Comment 3 Ricky Mondello 2013-11-11 18:09:36 PST
Created attachment 216628 [details]
Another attempt; might work for 10.8
Comment 4 Build Bot 2013-11-11 18:36:53 PST
Comment on attachment 216627 [details]
First attempt to run through EWS.

Attachment 216627 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/22369517
Comment 5 Ricky Mondello 2013-11-11 22:25:10 PST
Mark pointed out that my patch breaks the API contract (going as far as to explicitly edit a comment explaining it) of -[WebHistory orderedItemsLastVisitedOnDay:] and is therefore unacceptable. Specifically, `orderedItemsLastVisitedOnDay:` now takes an NSDate instead of an NSCalendarDate.
Comment 6 Ricky Mondello 2013-11-11 23:42:07 PST
Created attachment 216641 [details]
This time, don’t modify API.
Comment 7 Conrad Shultz 2013-11-12 00:35:56 PST
I'm not a reviewer, but the date math and API usage look correct to me.
Comment 8 Anders Carlsson 2013-11-12 06:21:34 PST
Comment on attachment 216641 [details]
This time, don’t modify API.

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

> Source/WebKit/mac/History/WebHistory.h:131
> +#pragma clang diagnostic push
> +#pragma clang diagnostic ignored "-Wdeprecated-declarations"
>  - (NSArray *)orderedItemsLastVisitedOnDay:(NSCalendarDate *)calendarDate;
> +#pragma clang diagnostic pop

We can't disable deprecation warnings in public headers!
Comment 9 Ricky Mondello 2013-11-12 12:50:28 PST
Created attachment 216705 [details]
Use NS_DEPRECATED_MAC instead of #pragma-ing API and code supporting API
Comment 10 Build Bot 2013-11-12 13:24:05 PST
Comment on attachment 216705 [details]
Use NS_DEPRECATED_MAC instead of #pragma-ing API and code supporting API

Attachment 216705 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/22888797
Comment 11 Build Bot 2013-11-12 13:32:39 PST
Comment on attachment 216705 [details]
Use NS_DEPRECATED_MAC instead of #pragma-ing API and code supporting API

Attachment 216705 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/22369739
Comment 12 Andy Estes 2013-11-12 16:40:26 PST
Comment on attachment 216705 [details]
Use NS_DEPRECATED_MAC instead of #pragma-ing API and code supporting API

NSCalendarDate was also deprecated in iOS 7. Instead of using NS_DEPRECATED_MAC(10_4, 10_9), you should use NS_DEPRECATED(10_4, 10_9, 2_0, 7_0).
Comment 13 Andy Estes 2013-11-13 17:04:59 PST
<rdar://problem/15462774>
Comment 14 Mark Rowe (bdash) 2013-11-18 20:36:47 PST
Created attachment 217267 [details]
Disable the deprecation warnings

This change in WebHistory.h isn't ideal, but I'm really bored with having to work around this. We can remove it when we address bug 124189.
Comment 15 WebKit Commit Bot 2013-11-18 20:38:10 PST
Attachment 217267 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/mac/ChangeLog', u'Source/WebKit/mac/History/WebHistory.h', u'Source/WebKit/mac/History/WebHistory.mm']" exit_code: 1
Source/WebKit/mac/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 16 WebKit Commit Bot 2013-11-18 21:12:16 PST
Comment on attachment 217267 [details]
Disable the deprecation warnings

Clearing flags on attachment: 217267

Committed r159480: <http://trac.webkit.org/changeset/159480>
Comment 17 WebKit Commit Bot 2013-11-18 21:12:19 PST
All reviewed patches have been landed.  Closing bug.