Merge lp:~gue5t/midori/completion-escape-hist into lp:midori

[gue5t gue5t]

Don't entity-escape history and bookmark results in location completion

Changes a flag to indicate that history and bookmark results in location completion are markup.

Fixes #1435614.

[Cris Dywan (kalikiana)]

Could we have a unit test for this? We have some that test the result of rendering, and also based on fake data.

review: Needs Fixing

[lp:~gue5t/midori/completion-escape-hist]

6908. By gue5t <email address hidden> on 2015-04-04

add a test to ensure that page titles from history are properly shown in completion

[gue5t gue5t (gue5t)]

The change to midori-completion.vala (replacing .begin() with yield) is necessary because using .begin() in a function that's already async makes it impossible to use regular Vala "yield" to wait for the nested asynchronous action. It doesn't cause async code to become synchronous, since the yield itself is already in an async method.

[Cris Dywan (kalikiana)]

That seems sensible - I think it was done this way in an attempt to allow multiple completions to run at once, but that might have been racy to begin with.

Thanks a lot for adding the test!

review: Approve

=== modified file 'midori/midori-completion.vala'
--- midori/midori-completion.vala	2014-03-16 12:15:50 +0000
+++ midori/midori-completion.vala	2015-04-04 19:22:35 +0000
@@ -168,7 +168,7 @@
 
             foreach (var completion in completions) {
                 if (completion.can_complete (text))
-                    complete_wrapped.begin (completion, text, null, cancellable);
+                    yield complete_wrapped (completion, text, null, cancellable);
             }
         }
 

=== modified file 'midori/midori-historycompletion.vala'
--- midori/midori-historycompletion.vala	2014-03-03 01:17:05 +0000
+++ midori/midori-historycompletion.vala	2015-04-04 19:22:35 +0000
@@ -11,7 +11,7 @@
 
 namespace Midori {
     public class HistoryCompletion : Completion {
-        HistoryDatabase? database = null;
+        public HistoryDatabase? database = null;
 
         public HistoryCompletion () {
             GLib.Object (description: _("Bookmarks and History"));
@@ -46,7 +46,7 @@
                 if (item is Midori.HistoryWebsite) {
                     var website = item as Midori.HistoryWebsite;
                     suggestions.append (new Suggestion (website.uri, website.title,
-                        false, null, yield Midori.URI.get_icon_fallback (website.uri, null, cancellable), this.position));
+                        true, null, yield Midori.URI.get_icon_fallback (website.uri, null, cancellable), this.position));
                 }
                 else if (item is Midori.HistorySearch) {
                     var search = item as Midori.HistorySearch;

=== modified file 'tests/completion.vala'
--- tests/completion.vala	2014-01-27 22:57:39 +0000
+++ tests/completion.vala	2015-04-04 19:22:35 +0000
@@ -134,6 +134,61 @@
     }
 }
 
+class HistoryMarkup : Midori.Test.Job {
+    public static void test () { new HistoryMarkup ().run_sync (); }
+    public override async void run (Cancellable cancellable) throws GLib.Error {
+        var app = new Midori.App ();
+        var autocompleter = new Midori.Autocompleter (app);
+        assert (!autocompleter.can_complete (""));
+
+        var histcomp = new Midori.HistoryCompletion ();
+        assert (!histcomp.can_complete (""));
+
+        //this calls histcomp.prepare (app):
+        autocompleter.add (histcomp);
+
+        //any time the history completion has a db, its can_complete method returns true
+        //assert (!histcomp.can_complete (""));
+
+        //remove entries from previous tests
+        histcomp.database.clear (0);
+
+        histcomp.database.insert ("https://duckduckgo.com/?q=%3E&ia=about", "> (Clojure) - DuckDuckGo", 0, 0);
+        yield autocompleter.complete ("");
+        assert (autocompleter.model.iter_n_children (null) == 2);
+
+        histcomp.database.insert ("https://duckduckgo.com/", "DuckDuckGo", 0, 0);
+        yield autocompleter.complete ("");
+        assert (autocompleter.model.iter_n_children (null) == 3);
+
+        histcomp.database.insert ("http://stackoverflow.com/questions/5068951/what-do-lt-and-gt-stand-for",
+            "html - What do < and > stand for? - Stack Overflow", 0, 0);
+        yield autocompleter.complete ("");
+        assert (autocompleter.model.iter_n_children (null) == 4);
+
+        Gtk.TreeIter iter;
+        string title, expected;
+
+        expected = "DuckDuckGo";
+        autocompleter.model.iter_nth_child (out iter, null, 2);
+        autocompleter.model.get (iter, Midori.Autocompleter.Columns.MARKUP, out title);
+        if (title != expected)
+            error ("Expected %s but got %s", expected, title);
+
+        expected = "> (Clojure) - DuckDuckGo";
+        autocompleter.model.iter_nth_child (out iter, null, 3);
+        autocompleter.model.get (iter, Midori.Autocompleter.Columns.MARKUP, out title);
+        if (title != expected)
+            error ("Expected %s but got %s", expected, title);
+
+        expected = "html - What do < and > stand for? - Stack Overflow";
+        autocompleter.model.iter_nth_child (out iter, null, 1);
+        autocompleter.model.get (iter, Midori.Autocompleter.Columns.MARKUP, out title);
+        if (title != expected)
+            error ("Expected %s but got %s", expected, title);
+    }
+}
+
 void main (string[] args) {
     Midori.Test.init (ref args);
     Midori.App.setup (ref args, null);
@@ -141,6 +196,7 @@
     Test.add_func ("/completion/autocompleter", CompletionAutocompleter.test);
     Test.add_func ("/completion/history", CompletionHistory.test);
     Test.add_func ("/completion/location-action", completion_location_action);
+    Test.add_func ("/completion/historymarkup", HistoryMarkup.test);
     Test.run ();
 }