Implement the SearchPopupMenu for Qt using QSettings.
Created attachment 30185 [details] Implementation using QSettings Feedback welcome, specially on the ::enabled implementation
My only concern is that the code will save something to QSettings behind the user's back. This can trip something w.r.t to the privacy issues. An alternative will be to use QWebSettings, this needs adding a pointer to the Frame somewhere in the code path.
Created attachment 30334 [details] Add SearchPopupMenuDelegate to bridge to the Kit Add Document* to the signature (like we did for Cookies), and pick *Delegate as the name as we have a PopupMenuClient and a SearchPopupMenuClient would be different. The Delegate approach was used for the Clipboard implementation on Gtk+ as well. Also start using the new #if USE() instead of having PLATFORM(GTK) && PLATFORM(QT) and share the generic part.
Created attachment 30335 [details] Implementation for Qt using QSettings and QWebSettings First attempt on the API layer, simply add an attribute to QWebSettings and rely on QSettings... document the usage of QSettings() and the need for proper app and org name,
Comment on attachment 30334 [details] Add SearchPopupMenuDelegate to bridge to the Kit Looks okay but needs a changelog. Also the GTK change doesn't look to be related?
Comment on attachment 30335 [details] Implementation for Qt using QSettings and QWebSettings > + foreach(QString string, urls) > + searchItems.append(string); That should be const QString, right? Beyond that, I agree with Ariya that maybe this should not be saved in QSettings for privacy reasons.
Created attachment 30605 [details] Add SearchPopupMenuDelegate to bridge to the Kit now with ChangeLog Same patch with a ChangeLog entry, explaining the motive, the reason to name it Delegate and the plan to make use of it in Gtk+ and Qt.
Created attachment 30606 [details] Implementation for Qt using QSettings and QWebSettings with ChangeLog So far only changed to const QString as requested by George. My take on privacy is the following: - It is off by default - The "cached" queries are never revealed to HTML/JS - It is using QSettings(appName, orgName) so outside of the app it is not used. For hosts like dashboard/plasma if you open the google search in two plasmoids this google field will have a shared history... I think that is feature and not a security issue?
I agree with Zecke and don't see any larger issues with privacy, as it is disabled by default. Still it feels a big strange storing this are settings, but maybe that is just me.
118 static void initializeWebCoreIfNeeded() 119 { 120 static bool called = false; 121 if (!called) 122 return; 123 124 called = true; I believe that should be 'if (called) no? Also, I think you've addressed the privacy issues, but might I suggest adding API so clients can very easily delete the stored settings much like they would do when clearing cookies, history, etc?
Comment on attachment 30606 [details] Implementation for Qt using QSettings and QWebSettings with ChangeLog > + past search queries should be used for search fields. The search queries will be > + store with QSettings, for this to work make sure to call Minor nit, but that should be 'stored' I believe. > void SearchPopupMenuDelegate::saveRecentSearches(const AtomicString& name, const Vector<String>& searchItems, Document*) > { > + if (name.isEmpty()) > + return; > + > + // put into the QStringList > + QStringList urls; > + for (int i = 0; i < searchItems.size(); ++i) > + urls.append(searchItems[i]); Shouldn't we have a maximum number of entries here? Adam
Is this still valid? Long time since the last update here...