[Merge] lp:~rpadovani/webbrowser-app/remove-404-history into lp:webbrowser-app

Olivier Tilloy olivier.tilloy at canonical.com
Wed May 27 10:18:26 UTC 2015


Review: Needs Fixing

I’m seeing a few issues with the test:

 - Could you make it browse to a valid page that returns a 200 code after waiting for 404page to be loaded? This would ensure that the fact that the 404 page is not present in the history view is not a timing issue. (sorry this contradicts my initial suggestion to not browse to another page…)

 - HistoryView.get_domain_entries() should return entries ordered by position. Autopilot doesn’t offer any guarantee as to the order in which objects are returned, so we need to sort them. See e.g. how this is done for Suggestions.get_ordered_entries().

 - The ExpandedHistoryView component has an additional UrlDelegate instance in its header, so [delegates = expanded_history.select_many("UrlDelegate")] will return one item too many. You should give the delegates an objectName, and call select_many() with that objectName as a parameter.

-- 
https://code.launchpad.net/~rpadovani/webbrowser-app/remove-404-history/+merge/259313
Your team Ubuntu Phablet Team is subscribed to branch lp:webbrowser-app.



More information about the Ubuntu-reviews mailing list