<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - We should be able to clear search field recent searches based on time given"
href="https://bugs.webkit.org/show_bug.cgi?id=150019#c2">Comment # 2</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - We should be able to clear search field recent searches based on time given"
href="https://bugs.webkit.org/show_bug.cgi?id=150019">bug 150019</a>
from <span class="vcard"><a class="email" href="mailto:mitz@webkit.org" title="mitz@webkit.org <mitz@webkit.org>"> <span class="fn">mitz@webkit.org</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=262876&action=diff" name="attach_262876" title="Patch">attachment 262876</a> <a href="attachment.cgi?id=262876&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=262876&action=review">https://bugs.webkit.org/attachment.cgi?id=262876&action=review</a>
<span class="quote">> Source/WebCore/platform/cocoa/SearchPopupMenuCocoa.h:35
> +WEBCORE_EXPORT void removeSearchFieldRecentSearchesModifiedSince(std::chrono::system_clock::time_point modifiedSince);</span >
This can probably just be a static function in SearchPopupMenu, and then it can have a shorter name.
<span class="quote">> Source/WebCore/platform/cocoa/SearchPopupMenuCocoa.mm:148
> + NSMutableDictionary *emptyRecentSearchesDictionary = [NSMutableDictionary dictionaryWithObjectsAndKeys:[NSMutableDictionary dictionary], itemsKey, nil];
> + [emptyRecentSearchesDictionary writeToFile:searchFieldRecentSearchesPlistPath() atomically:NO];</span >
Can we just delete this file instead of writing an empty file? Does the presence of a file signify something? Does it need to include this key-value pair? Why is everything here mutable?
<span class="quote">> Source/WebCore/platform/cocoa/SearchPopupMenuCocoa.mm:149
> + }</span >
Did you mean to return early after this?
<span class="quote">> Source/WebCore/platform/cocoa/SearchPopupMenuCocoa.mm:153
> + if (![[itemsDictionary classForCoder] isSubclassOfClass:[NSMutableDictionary class]])</span >
What is the significance of checking this way vs. the -isKindOfClass: checks below?
<span class="quote">> Source/WebCore/platform/cocoa/SearchPopupMenuCocoa.mm:158
> + if (![key isKindOfClass:[NSString class]])</span >
I mean this…
<span class="quote">> Source/WebCore/platform/cocoa/SearchPopupMenuCocoa.mm:162
> + if (![[nameDictionary classForCoder] isSubclassOfClass:[NSMutableDictionary class]])</span >
…versus this.
<span class="quote">> Source/WebKit2/Shared/WebsiteData/WebsiteDataTypes.h:48
> + WebsiteDataTypeSearchFieldRecentSearches = 1 << 10,
> #if ENABLE(NETSCAPE_PLUGIN_API)
> - WebsiteDataTypePlugInData = 1 << 10,
> + WebsiteDataTypePlugInData = 1 << 11,
> #endif
> #if ENABLE(MEDIA_STREAM)
> - WebsiteDataTypeMediaDeviceIdentifier = 1 << 11,
> + WebsiteDataTypeMediaDeviceIdentifier = 1 << 12,
> #endif</span >
Why insert this in the middle?
<span class="quote">> Source/WebKit2/UIProcess/API/Cocoa/WKWebsiteDataRecordPrivate.h:32
> +WK_EXTERN NSString * const _WKWebsiteDataTypeSearchFieldRecentSearches WK_AVAILABLE(10_11, 9_0);</span >
This is not present in 10.11 and 9.0. You should use WK_MAC_TBA and WK_IOS_TBA instead.
<span class="quote">> Source/WebKit2/UIProcess/WebsiteData/WebsiteDataStore.cpp:40
> +#if PLATFORM(COCOA)
> +#include <WebCore/SearchPopupMenuCocoa.h>
> +#endif</span >
Conditional imports go in a separate paragraph after the main block of imports.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>