webbrowser-app

Merge lp:~uriboni/webbrowser-app/bookmark-folders-from-contextual-menu into lp:webbrowser-app

bookmark-folders-from-contextual-menu
Merge into trunk

Proposed by Ugo Riboni on 2015-08-13

Status:

Merged

Approved by:

Olivier Tilloy on 2015-09-03

Approved revision:

1144

Merged at revision:

1164

Proposed branch:

lp:~uriboni/webbrowser-app/bookmark-folders-from-contextual-menu

Merge into:

lp:webbrowser-app

Diff against target:

492 lines (+147/-74)

10 files modified

src/app/UrlUtils.js (+10/-5)
src/app/webbrowser/AddressBar.qml (+2/-1)
src/app/webbrowser/Browser.qml (+23/-10)
src/app/webbrowser/Chrome.qml (+3/-0)
src/app/webbrowser/NavigationBar.qml (+2/-0)
tests/autopilot/webbrowser_app/emulators/browser.py (+36/-0)
tests/autopilot/webbrowser_app/tests/__init__.py (+2/-1)
tests/autopilot/webbrowser_app/tests/test_bookmark_options.py (+35/-5)
tests/autopilot/webbrowser_app/tests/test_contextmenu.py (+20/-52)
tests/unittests/qml/tst_UrlUtils.qml (+14/-0)

To merge this branch:

bzr merge lp:~uriboni/webbrowser-app/bookmark-folders-from-contextual-menu

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot	continuous-integration		Needs Fixing on 2015-09-03
Olivier Tilloy		2015-08-13	Approve on 2015-09-03
Review via email: mp+267934@code.launchpad.net

Commit message

Allow choosing the bookmark folder when bookmarking a link from the context menu. Disable the bookmark option when the link is already bookmarked. Ensure the bookmark star state in the chrome is always consistent with the bookmarked state of the current webview URL.

Description of the change

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-08-13:

FAILED: Continuous integration, rev:1123
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2082/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/3685/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/836/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/836/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/836/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/3682/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2082/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-08-13:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2087/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-08-14:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2090/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-08-14:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2094/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-08-14:

Please revert changes to po/webbrowser-app.pot.

review: Needs Fixing

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-08-17:

The revert of po/webbrowser-app.pot was done from an old version of the trunk, so when merging into the latest trunk there are still changes to that file.

If a hyperlink doesn’t have a title, the title field will appear empty in the popover. In that case I think it would make sense to pre-fill the field with the URL. What do you think?

contextualMenuTarget should be made explicitly invisible, and its x, y, width and height property could be bound to contextualData.sourceArea’s values, instead of setting them imperatively in the action’s onTriggered handler.

Autopilot tests:

self.pointing_device.click_object(webview, button=3) works only on desktop, right? The test needs to work on devices as well, by simulating a long press on the page. You can do something like that:

    webview = self.main_window.get_current_webview()
    self.pointing_device.move_to_object(webview)
    if model() == 'Desktop':
        self.pointing_device.click(button=3)
    else:
        self.pointing_device.press()
        time.sleep(1.5)
        self.pointing_device.release()

actions.click_button_by_text("Bookmark link") isn’t portable. It fails on my desktop because the text is translated. A portable way is to add a name to the actions (e.g. BookmarkLink.qml would have [name: "bookmarklink"], all actions need to have a unique name), and in UbuntuWebView02.qml add a custom delegate for the ActionSelectionPopover whose objectName will be set to 'action.name' (plus optionally a prefix/suffix). Then you can select the menu item by objectName.

review: Needs Fixing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-08-17:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2105/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-08-17:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2113/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-08-18:

FAILED: Continuous integration, rev:1131
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2118/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/3780/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/872/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/872/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/872/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/3777/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2118/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Ugo Riboni (uriboni) wrote on 2015-08-18:

All done except when otherwise noted below:

> If a hyperlink doesn’t have a title, the title field will appear empty in the
> popover. In that case I think it would make sense to pre-fill the field with
> the URL. What do you think?

Design said we should display the URL without the scheme (e.g. without http://), and do that in all cases when the popover for bookmark options appear, so I did just that.

> actions.click_button_by_text("Bookmark link") isn’t portable. It fails on my
> desktop because the text is translated. A portable way is to add a name to the
> actions (e.g. BookmarkLink.qml would have [name: "bookmarklink"], all actions
> need to have a unique name), and in UbuntuWebView02.qml add a custom delegate
> for the ActionSelectionPopover whose objectName will be set to 'action.name'
> (plus optionally a prefix/suffix). Then you can select the menu item by
> objectName.

Adding a custom delegate is a workaround, but the right fix is for the ActionSelectionPopover to do this by default (otherwise they would essentially be making mandatory to have your own delegate, if you want to write testable code).
I have been talking to the SDK people about this and they agree on this fix.

There is a bug to track all this https://bugs.launchpad.net/ubuntu-ui-toolkit/+bug/1205144 and there is a patch attached to the bug that Christian created a few days ago.

It seems to me that using a custom delegate is adding an awful amount of code just for testing purposes, and we have been trying very hard to avoid this in the past. I would suggest that we fix the bug as if the code in the UITK fix has been committed, then disable the test with a link to the bug, then re-enabled it when it has been committed for real.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-08-18:

FAILED: Continuous integration, rev:1132
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2119/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/3781/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/873/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/873/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/873/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/3778/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2119/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-08-18:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2120/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-08-18:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2124/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-08-18:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2126/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-08-19:

Looks good to me, thanks!

review: Approve

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-09-01:

There are now a bunch of conflicts when merging this branch into the latest trunk, would you mind addressing them? Thanks!

review: Needs Fixing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-09-01:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2153/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-09-02:

test_bookmark_options_from_contextual_menu can now be updated and unskipped, since the context menu implementation has custom delegates (and the emulator has a get_context_menu() method, which returns an object that has a click_action() method).

The _invoke_contextual_menu_on_item() could be made an emulator method, since it duplicates code in test_contextmenu.py (have a look at the open_context_menu() method there, which takes into account the height of the chrome to determine where to invoke the context menu).

In UrlUtils.js, the naming of the 'authority' variable in the new removeScheme function is confusing, since it’s not actually extracting the authority, only removing the scheme. Call it 'remainder', or 'tail', or something more accurate please.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-09-02:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2156/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-09-02:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2160/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-09-02:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2161/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-09-02:

+ id: tab

This doesn’t seem to be used anywhere, can the change be reverted?

The _invoke_contextual_menu_on_item helper method isn’t used anymore, it should be removed.

Other than that, LGTM.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-09-02:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2163/rebuild

review: Approve (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) on 2015-09-03:

review: Approve

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-09-03:

flake8 unit test is failing

review: Needs Fixing

lp:~uriboni/webbrowser-app/bookmark-folders-from-contextual-menu updated on 2015-09-03

1144. By Ugo Riboni on 2015-09-03: Fix flake8

Revision history for this message

Olivier Tilloy (osomon) on 2015-09-03:

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-09-03:

FAILED: Continuous integration, rev:1143
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2169/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/3992/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/923/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/923/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/923/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/3989/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2169/rebuild

review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Ubuntu Phablet Team

Ugo Riboni

to status/vote changes:

Rachel Liu

 === modified file 'src/app/UrlUtils.js'
 --- src/app/UrlUtils.js	2015-06-16 16:32:39 +0000
 +++ src/app/UrlUtils.js	2015-09-03 09:40:58 +0000
@@ -18,12 +18,17 @@
  'use strict';
++function removeScheme(url) {
++    var rest = url.toString()
++    var indexOfScheme = rest.indexOf("://")
++    if (indexOfScheme !== -1) {
++        rest = rest.slice(indexOfScheme + 3)
++    }
++    return rest
++}
++
  function extractAuthority(url) {
--    var authority = url.toString()
--    var indexOfScheme = authority.indexOf("://")
--    if (indexOfScheme !== -1) {
--        authority = authority.slice(indexOfScheme + 3)
--    }
++    var authority = removeScheme(url)
      var indexOfPath = authority.indexOf("/")
      if (indexOfPath !== -1) {
          authority = authority.slice(0, indexOfPath)
 === modified file 'src/app/webbrowser/AddressBar.qml'
 --- src/app/webbrowser/AddressBar.qml	2015-08-18 08:49:24 +0000
 +++ src/app/webbrowser/AddressBar.qml	2015-09-03 09:40:58 +0000
@@ -30,6 +30,7 @@
      property bool incognito: false
      property alias text: textField.text
      property bool bookmarked: false
++    signal toggleBookmark()
      property url requestedUrl
      property url actualUrl
      signal validated()
@@ -223,7 +224,7 @@
                      color: addressbar.bookmarked ? UbuntuColors.orange : UbuntuColors.darkGrey
+                 }
--                onClicked: addressbar.bookmarked = !addressbar.bookmarked
++                onClicked: addressbar.toggleBookmark()
                  Item {
                      id: bookmarkTogglePlaceHolderItem
 === modified file 'src/app/webbrowser/Browser.qml'
 --- src/app/webbrowser/Browser.qml	2015-08-27 14:02:05 +0000
 +++ src/app/webbrowser/Browser.qml	2015-09-03 09:40:58 +0000
@@ -316,12 +316,9 @@
                  return ((webview && browser.bookmarksModel) ? browser.bookmarksModel.contains(webview.url) : false)
+             }
              bookmarked: isCurrentUrlBookmarked()
--            onBookmarkedChanged: {
--                if (bookmarked && !isCurrentUrlBookmarked()) {
--                    internal.addBookmark(webview.url, webview.title, webview.icon)
--                } else if (!bookmarked && isCurrentUrlBookmarked()) {
--                    browser.bookmarksModel.remove(webview.url)
--                }
++            onToggleBookmark: {
++                if (isCurrentUrlBookmarked()) browser.bookmarksModel.remove(webview.url)
++                else internal.addBookmark(webview.url, webview.title, webview.icon)
+             }
              onWebviewChanged: bookmarked = isCurrentUrlBookmarked()
              Connections {
@@ -903,6 +900,11 @@
              current: tabsModel && tabsModel.currentTab === this
              focus: current
++            Item {
++                id: contextualMenuTarget
++                visible: false
++            }
++
              webviewComponent: WebViewImpl {
                  id: webviewimpl
@@ -942,8 +944,17 @@
                      Actions.BookmarkLink {
                          objectName: "bookmarkLinkContextualAction"
                          enabled: contextModel && contextModel.linkUrl.toString() &&
--                                 browser.bookmarksModel
--                        onTriggered: bookmarksModel.add(contextModel.linkUrl, contextModel.linkText, "", "")
++                                 browser.bookmarksModel && !bookmarksModel.contains(contextModel.linkUrl)
++                        onTriggered: {
++                            // position the menu target with a one-off assignement instead of a binding
++                            // since the contents of the contextModel have meaning only while the context
++                            // menu is active
++                            contextualMenuTarget.x = contextModel.position.x * devicePixelRatio
++                            contextualMenuTarget.y = contextModel.position.y * devicePixelRatio +
++                                                     locationBarController.height + locationBarController.offset
++                            internal.addBookmark(contextModel.linkUrl, contextModel.linkText,
++                                                 "", contextualMenuTarget)
++                        }
+                     }
                      Actions.CopyLink {
                          objectName: "copyLinkContextualAction"
@@ -1269,10 +1280,12 @@
+             }
+         }
--        function addBookmark(url, title, icon) {
++        function addBookmark(url, title, icon, location) {
++            if (title == "") title = UrlUtils.removeScheme(url)
              bookmarksModel.add(url, title, icon, "")
++            if (location === undefined) location = chrome.bookmarkTogglePlaceHolder
              PopupUtils.open(bookmarkOptionsComponent,
--                            chrome.bookmarkTogglePlaceHolder,
++                            location,
                              {"bookmarkUrl": url,
                               "bookmarkTitle": title})
+         }
 === modified file 'src/app/webbrowser/Chrome.qml'
 --- src/app/webbrowser/Chrome.qml	2015-08-11 10:37:56 +0000
 +++ src/app/webbrowser/Chrome.qml	2015-09-03 09:40:58 +0000
@@ -27,6 +27,7 @@
      property alias searchUrl: navigationBar.searchUrl
      property alias text: navigationBar.text
      property alias bookmarked: navigationBar.bookmarked
++    signal toggleBookmark()
      property alias drawerActions: navigationBar.drawerActions
      property alias drawerOpen: navigationBar.drawerOpen
      property alias requestedUrl: navigationBar.requestedUrl
@@ -90,6 +91,8 @@
                  right: parent.right
+             }
              height: units.gu(6)
++
++            onToggleBookmark: chrome.toggleBookmark()
+         }
+     }
+ }
 === modified file 'src/app/webbrowser/NavigationBar.qml'
 --- src/app/webbrowser/NavigationBar.qml	2015-08-11 10:37:56 +0000
 +++ src/app/webbrowser/NavigationBar.qml	2015-09-03 09:40:58 +0000
@@ -27,6 +27,7 @@
      property alias searchUrl: addressbar.searchUrl
      readonly property string text: addressbar.text
      property alias bookmarked: addressbar.bookmarked
++    signal toggleBookmark()
      property list<Action> drawerActions
      readonly property bool drawerOpen: internal.openDrawer
      property alias requestedUrl: addressbar.requestedUrl
@@ -126,6 +127,7 @@
                  webview.reload()
+             }
              onRequestStop: webview.stop()
++            onToggleBookmark: root.toggleBookmark()
              Connections {
                  target: webview
 === modified file 'tests/autopilot/webbrowser_app/emulators/browser.py'
 --- tests/autopilot/webbrowser_app/emulators/browser.py	2015-08-25 13:56:58 +0000
 +++ tests/autopilot/webbrowser_app/emulators/browser.py	2015-09-03 09:40:58 +0000
@@ -15,11 +15,13 @@
  # along with this program. If not, see <http://www.gnu.org/licenses/>.
  import logging
++import time
  import autopilot.logging
  import ubuntuuitoolkit as uitk
  from autopilot import exceptions
  from autopilot import input
++from autopilot.platform import model
  logger = logging.getLogger(__name__)
@@ -174,6 +176,35 @@
          else:
              return self.wait_select_single(ContextMenuMobile)
++    def open_context_menu(self):
++        webview = self.get_current_webview()
++        chrome = self.chrome
++        x = webview.globalRect.x + webview.globalRect.width // 2
++        y = webview.globalRect.y + \
++            (webview.globalRect.height + chrome.height) // 2
++        self.pointing_device.move(x, y)
++        if model() == 'Desktop':
++            self.pointing_device.click(button=3)
++        else:
++            self.pointing_device.press()
++            time.sleep(1.5)
++            self.pointing_device.release()
++        return self.get_context_menu()
++
++    def dismiss_context_menu(self, menu):
++        if self.wide:
++            # Dismiss by clicking outside of the menu
++            webview_rect = self.get_current_webview().globalRect
++            actions = menu.get_visible_actions()
++            outside_x = (webview_rect.x + actions[0].globalRect.x) // 2
++            outside_y = webview_rect.y + webview_rect.height // 2
++            self.pointing_device.move(outside_x, outside_y)
++            self.pointing_device.click()
++        else:
++            # Dismiss by clicking the cancel action
++            menu.click_cancel_action()
++        menu.wait_until_destroyed()
++
  class Chrome(uitk.UbuntuUIToolkitCustomProxyObjectBase):
@@ -574,11 +605,16 @@
      def get_visible_actions(self):
          return self.select_many("Empty", visible=True)
++    def get_action(self, objectName):
++        name = objectName + "_item"
++        return self.select_single("Empty", objectName=name)
++
      def click_action(self, objectName):
          name = objectName + "_item"
          action = self.select_single("Empty", visible=True,
                                      enabled=True, objectName=name)
          self.pointing_device.click_object(action)
++        self.wait_until_destroyed()
  class ContextMenuWide(ContextMenuBase):
 === modified file 'tests/autopilot/webbrowser_app/tests/__init__.py'
 --- tests/autopilot/webbrowser_app/tests/__init__.py	2015-08-12 12:31:56 +0000
 +++ tests/autopilot/webbrowser_app/tests/__init__.py	2015-09-03 09:40:58 +0000
@@ -233,7 +233,8 @@
          self.useFixture(fixtures.EnvironmentVariable(
              'UBUNTU_WEBVIEW_HOST_MAPPING_RULES',
              "MAP test:80 localhost:{}".format(self.http_server.port)))
--        self.base_url = "http://test"
++        self.base_domain = "test"
++        self.base_url = "http://" + self.base_domain
          self.url = self.base_url + path
          self.ARGS = self.ARGS + [self.url]
          super(StartOpenRemotePageTestCaseBase, self).setUp()
 === modified file 'tests/autopilot/webbrowser_app/tests/test_bookmark_options.py'
 --- tests/autopilot/webbrowser_app/tests/test_bookmark_options.py	2015-08-15 00:30:14 +0000
 +++ tests/autopilot/webbrowser_app/tests/test_bookmark_options.py	2015-09-03 09:40:58 +0000
@@ -17,7 +17,6 @@
  import os.path
  import sqlite3
  import time
--import testtools
  from autopilot.matchers import Eventually
  from testtools.matchers import Equals
@@ -201,10 +200,8 @@
                          Eventually(Equals(4)))
          self._assert_bookmark_count_in_folder(new_tab, "NewFolder", 1)
--    @testtools.skip("Temporarily skipped until popover going out of view with"
--                    " OSK is fixed http://pad.lv/1466222")
      def test_set_bookmark_title(self):
--        url = self.base_url + "/test2"
++        url = self.base_url + "/blanktargetlink"
          self.main_window.go_to_url(url)
          self.main_window.wait_until_page_loaded(url)
@@ -214,6 +211,8 @@
          bookmark_options = self._get_bookmark_options()
          title_text_field = bookmark_options.get_title_text_field()
++        self.assertThat(title_text_field.text,
++                        Equals(self.base_domain + "/blanktargetlink"))
          self.pointing_device.click_object(title_text_field)
          title_text_field.activeFocus.wait_for(True)
          title_text_field.write("NewTitle", True)
@@ -227,7 +226,38 @@
          self._assert_bookmark_count_in_folder(new_tab, "", 5)
          index = 0
--        if self.main_view.wide:
++        if self.main_window.wide:
              index += 1
          bookmark = new_tab.get_bookmarks("")[index]
          self.assertThat(bookmark.title, Equals("NewTitle"))
++
++    def test_bookmark_options_from_contextual_menu(self):
++        url = self.base_url + "/blanktargetlink"
++        self.main_window.go_to_url(url)
++        self.main_window.wait_until_page_loaded(url)
++        webview = self.main_window.get_current_webview()
++
++        # invoke the context menu over the link, which covers the entire page
++        menu = self.main_window.open_context_menu()
++        menu.click_action("bookmarkLinkContextualAction")
++
++        bookmark_options = self.main_window.get_bookmark_options()
++        bookmark_options.click_dismiss_button()
++        bookmark_options.wait_until_destroyed()
++
++        # reopen the context menu and verify that the bookmark options is
++        # disabled as we have already bookmarked this link
++        menu = self.main_window.open_context_menu()
++        bookmark_action = menu.get_action("bookmarkLinkContextualAction")
++        self.assertThat(bookmark_action.visible, Equals(False))
++
++        # dismiss the dialog
++        self.main_window.dismiss_context_menu(menu)
++
++        # click on the link and verify that the bookmark star is lit on the
++        # target page
++        self.pointing_device.click_object(webview)
++        self.main_window.wait_until_page_loaded(self.base_url + "/test2")
++
++        chrome = self.main_window.chrome
++        self.assertThat(chrome.bookmarked, Eventually(Equals(True)))
 === modified file 'tests/autopilot/webbrowser_app/tests/test_contextmenu.py'
 --- tests/autopilot/webbrowser_app/tests/test_contextmenu.py	2015-08-21 17:02:09 +0000
 +++ tests/autopilot/webbrowser_app/tests/test_contextmenu.py	2015-09-03 09:40:58 +0000
@@ -14,11 +14,10 @@
  # You should have received a copy of the GNU General Public License
  # along with this program. If not, see <http://www.gnu.org/licenses/>.
--import time
  from autopilot.platform import model
  from autopilot.matchers import Eventually
  import testtools
--from testtools.matchers import Equals, GreaterThan, StartsWith
++from testtools.matchers import Equals, StartsWith
  from webbrowser_app.tests import StartOpenRemotePageTestCaseBase
@@ -29,26 +28,7 @@
      def setUp(self, path):
          super(TestContextMenuBase, self).setUp(path)
--        self.menu = self.open_context_menu()
--
--    def open_context_menu(self):
--        webview = self.main_window.get_current_webview()
--        chrome = self.main_window.chrome
--        x = webview.globalRect.x + webview.globalRect.width // 2
--        y = webview.globalRect.y + \
--            (webview.globalRect.height + chrome.height) // 2
--        self.pointing_device.move(x, y)
--        if model() == 'Desktop':
--            self.pointing_device.click(button=3)
--        else:
--            self.pointing_device.press()
--            time.sleep(1.5)
--            self.pointing_device.release()
--        return self.main_window.get_context_menu()
--
--    def click_action(self, name):
--        self.menu.click_action(name)
--        self.menu.wait_until_destroyed()
++        self.menu = self.main_window.open_context_menu()
      def verify_link_opened_in_a_new_tab(self):
          self.assert_number_webviews_eventually(2)
@@ -76,38 +56,24 @@
          self.assertThat(self.menu.get_title_label().text,
                          Equals(self.base_url + "/test1"))
--    def test_dismiss_menu(self):
--        if self.main_window.wide:
--            # Verify that clicking outside the menu dismisses it
--            webview_rect = self.main_window.get_current_webview().globalRect
--            actions = self.menu.get_visible_actions()
--            self.assertThat(actions[0].globalRect.x,
--                            GreaterThan(webview_rect.x))
--            outside_x = (webview_rect.x + actions[0].globalRect.x) // 2
--            outside_y = webview_rect.y + webview_rect.height // 2
--            self.pointing_device.move(outside_x, outside_y)
--            self.pointing_device.click()
--        else:
--            # Verify that clicking the cancel action dismisses it
--            self.menu.click_cancel_action()
--        self.menu.wait_until_destroyed()
--
      def test_open_link_in_new_tab(self):
--        self.click_action("openLinkInNewTabContextualAction")
++        self.menu.click_action("openLinkInNewTabContextualAction")
          self.verify_link_opened_in_a_new_tab()
      def test_bookmark_link(self):
--        self.click_action("bookmarkLinkContextualAction")
++        self.menu.click_action("bookmarkLinkContextualAction")
++        bookmark_options = self.main_window.get_bookmark_options()
++        bookmark_options.click_dismiss_button()
          self.verify_link_bookmarked()
      def test_copy_link(self):
          # There is no easy way to test the contents of the clipboard,
          # but we can at least verify that the context menu was dismissed.
--        self.click_action("copyLinkContextualAction")
++        self.menu.click_action("copyLinkContextualAction")
      @testtools.skipIf(model() == "Desktop", "on devices only")
      def test_share_link(self):
--        self.click_action("ShareLinkContextualAction")
++        self.menu.click_action("ShareLinkContextualAction")
          self.main_window.wait_select_single("ContentShareDialog")
@@ -119,13 +85,13 @@
                          StartsWith(self.data_uri_prefix))
      def test_open_image_in_new_tab(self):
--        self.click_action("OpenImageInNewTabContextualAction")
++        self.menu.click_action("OpenImageInNewTabContextualAction")
          self.verify_image_opened_in_a_new_tab()
      def test_copy_image(self):
          # There is no easy way to test the contents of the clipboard,
          # but we can at least verify that the context menu was dismissed.
--        self.click_action("CopyImageContextualAction")
++        self.menu.click_action("CopyImageContextualAction")
  class TestContextMenuImageAndLink(TestContextMenuBase):
@@ -136,31 +102,33 @@
                          StartsWith(self.data_uri_prefix))
      def test_open_link_in_new_tab(self):
--        self.click_action("openLinkInNewTabContextualAction")
++        self.menu.click_action("openLinkInNewTabContextualAction")
          self.verify_link_opened_in_a_new_tab()
      def test_bookmark_link(self):
--        self.click_action("bookmarkLinkContextualAction")
++        self.menu.click_action("bookmarkLinkContextualAction")
++        bookmark_options = self.main_window.get_bookmark_options()
++        bookmark_options.click_dismiss_button()
          self.verify_link_bookmarked()
      def test_copy_link(self):
          # There is no easy way to test the contents of the clipboard,
          # but we can at least verify that the context menu was dismissed.
--        self.click_action("copyLinkContextualAction")
++        self.menu.click_action("copyLinkContextualAction")
      @testtools.skipIf(model() == "Desktop", "on devices only")
      def test_share_link(self):
--        self.click_action("ShareLinkContextualAction")
++        self.menu.click_action("ShareLinkContextualAction")
          self.main_window.wait_select_single("ContentShareDialog")
      def test_open_image_in_new_tab(self):
--        self.click_action("OpenImageInNewTabContextualAction")
++        self.menu.click_action("OpenImageInNewTabContextualAction")
          self.verify_image_opened_in_a_new_tab()
      def test_copy_image(self):
          # There is no easy way to test the contents of the clipboard,
          # but we can at least verify that the context menu was dismissed.
--        self.click_action("CopyImageContextualAction")
++        self.menu.click_action("CopyImageContextualAction")
  class TestContextMenuTextArea(TestContextMenuBase):
@@ -173,5 +141,5 @@
          actions = ["SelectAll", "Cut", "Undo", "Redo",
                     "Paste", "SelectAll", "Copy", "Erase"]
          for action in actions:
--            self.click_action("{}ContextualAction".format(action))
--            self.menu = self.open_context_menu()
++            self.menu.click_action("{}ContextualAction".format(action))
++            self.menu = self.main_window.open_context_menu()
 === modified file 'tests/unittests/qml/tst_UrlUtils.qml'
 --- tests/unittests/qml/tst_UrlUtils.qml	2015-04-27 20:37:41 +0000
 +++ tests/unittests/qml/tst_UrlUtils.qml	2015-09-03 09:40:58 +0000
@@ -55,6 +55,20 @@
          compare(UrlUtils.extractHost(data.url), data.host)
+     }
++    function test_removeScheme_data() {
++        return [
++            {url: "http://example.org/", removed: "example.org/"},
++            {url: "file://user:pwd@example.org:2442/", removed: "user:pwd@example.org:2442/"},
++            {url: "file:///home/foo/bar.txt", removed: "/home/foo/bar.txt"},
++            {url: "ht+tp://www.example.org/", removed: "www.example.org/"},
++            {url: "www.example.org", removed: "www.example.org"},
++        ]
++    }
++
++    function test_removeScheme(data) {
++        compare(UrlUtils.removeScheme(data.url), data.removed)
++    }
++
      function test_looksLikeAUrl_data() {
          return [
              {url: "", looksLike: false},