[Merge] lp:~artmello/webbrowser-app/webbrowser-app-bookmark_folders into lp:webbrowser-app
Olivier Tilloy
olivier.tilloy at canonical.com
Wed Jul 1 06:40:48 UTC 2015
Review: Needs Fixing
There are two remaining places in Browser.qml where a bookmark is created without the popover being displayed: HUD actions (which is defunct code, but until we remove it it should be kept up-to-date) and contextual actions.
The BookmarkOptions component should gain an additional 'url' property, whose value would be set to browser.currentWebview.url at construction time. This would ensure that the url doesn’t change while it’s shown. If the webview’s current URL changes while the popover is shown, the user can potentially try to unbookmark a different URL than the one that was bookmarked.
It’s wrong to skip test_set_bookmark_title. It’s not just the test that fails, it’s the actual functionality that’s broken. So we can’t land this branch until the UITK fix lands. By the way, have you tested the UITK branch (the fix appears to be merged in staging) to confirm that it fixes our issue?
If I press Ctrl+D to create a bookmark, then Ctrl+D again to remove it, the BookmarkOptions popover doesn’t disappear. If I then press Ctrl+D again to re-create the bookmark, I end up with two instances of the same popover.
--
https://code.launchpad.net/~artmello/webbrowser-app/webbrowser-app-bookmark_folders/+merge/260410
Your team Ubuntu Phablet Team is subscribed to branch lp:webbrowser-app.
More information about the Ubuntu-reviews
mailing list