Oxide

Test that WebContextMenu position property is in the correct coordinate space

Bug #1471181 reported by Chris Coulson
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Oxide
Fix Released
Medium
Olivier Tilloy
Oxide branch-1.10
1.9
Fix Released
Medium
Olivier Tilloy
Oxide 1.9.1

Bug Description

From looking at the code in Chromium that is used to bring up the context menu, it's not clear whether the position is in the source frame's coordinate space, or the WebView one. It's also not clear whether it's in physical or device-independent pixels.

It should be in the WebView coordinate space, and in physical pixels.

Testing this sort-of depends on bug 1399207, which will make it easier to add new test runs with arbitrary configurations (such as a non-unity device scale).

Related branches

lp:~osomon/webbrowser-app/oxide-context-menu
Ken VanDine: Approve
Ugo Riboni (community): Approve
PS Jenkins bot: Approve (continuous-integration)
Diff: 2579 lines (+1390/-690)
34 files modified
debian/control (+2/-12)
debian/qtdeclarative5-ubuntu-web-plugin-assets.install (+0/-1)
debian/rules (+0/-4)
doc/WebView.qdoc (+44/-5)
src/Ubuntu/Web/CMakeLists.txt (+0/-3)
src/Ubuntu/Web/Selection.qml (+0/-161)
src/Ubuntu/Web/SelectionHandle.qml (+0/-47)
src/Ubuntu/Web/UbuntuWebView02.qml (+60/-156)
src/Ubuntu/Web/selection02.js (+2/-131)
src/app/CMakeLists.txt (+1/-0)
src/app/FileExtensionMapper.js (+11/-7)
src/app/FilePickerDialog.qml (+2/-2)
src/app/WebViewImpl.qml (+20/-9)
src/app/actions/Cut.qml (+23/-0)
src/app/actions/Erase.qml (+23/-0)
src/app/actions/Paste.qml (+23/-0)
src/app/actions/Redo.qml (+23/-0)
src/app/actions/SaveLink.qml (+23/-0)
src/app/actions/SelectAll.qml (+23/-0)
src/app/actions/Undo.qml (+23/-0)
src/app/browserapplication.cpp (+9/-0)
src/app/mime-database.cpp (+33/-0)
src/app/mime-database.h (+39/-0)
src/app/webbrowser/Browser.qml (+103/-26)
src/app/webbrowser/ContextMenuMobile.qml (+168/-0)
src/app/webbrowser/ContextMenuWide.qml (+158/-0)
src/app/webbrowser/assets/stock_link.svg (+164/-0)
src/app/webcontainer/WebViewImplOxide.qml (+42/-5)
tests/autopilot/webbrowser_app/emulators/browser.py (+33/-16)
tests/autopilot/webbrowser_app/tests/http_server.py (+28/-0)
tests/autopilot/webbrowser_app/tests/test_contextmenu.py (+177/-0)
tests/autopilot/webbrowser_app/tests/test_selection.py (+0/-92)
tests/unittests/qml/tst_FileExtensionMapper.qml (+39/-0)
tests/unittests/qml/tst_UbuntuWebView02.qml (+94/-13)
lp:~osomon/oxide/contextmenu-position-physical-pixels
Chris Coulson: Pending requested
Diff: 24 lines (+6/-1)
1 file modified
qt/core/browser/oxide_qt_web_context_menu.cc (+6/-1)
lp:oxide/1.9
lp:ubuntu/wily/webbrowser-app
Chris Coulson (chrisccoulson)
Changed in oxide:
importance: Undecided → Medium
status: New → Triaged
Revision history for this message
Olivier Tilloy (osomon) wrote :

I just did some tests on a touch device, and it appears the position is expressed in device-independent pixels. One needs to multiply the coordinates by document.defaultView.devicePixelRatio to obtain a value in physical pixels.

Revision history for this message
Chris Coulson (chrisccoulson) wrote :

What about whether the position is relative to the origin of the webview, or of the frame?

Revision history for this message
Olivier Tilloy (osomon) wrote :

Not tested yet, will do when I get around to fixing the units the position is expressed in.

Revision history for this message
Olivier Tilloy (osomon) wrote :

I just verified that the position is relative to the origin of the webview, so that at least is correct.
Working on a fix for the units.

Changed in oxide:
status: Triaged → In Progress
assignee: nobody → Olivier Tilloy (osomon)
Revision history for this message
Olivier Tilloy (osomon) wrote :

Note that the coordinate space is already being tested by existing unit tests (see http://bazaar.launchpad.net/~oxide-developers/oxide/oxide.trunk/view/head:/qt/tests/qmltests/api/tst_WebView_contextMenu.qml#L64), where invoking a context menu on an iframe and verifying that the position corresponds to the position where the mouse was clicked, in webview coordinates.

The units (DIP vs physical pixels) is not being tested at the moment, it could probably be tested by setting the value of the OXIDE_FORCE_DPR environment variable to something ≠ 1.0.

Olivier Tilloy (osomon)
Changed in oxide:
status: In Progress → Fix Released
milestone: none → 1.9.1
milestone: 1.9.1 → branch-1.10
Revision history for this message
Olivier Tilloy (osomon) wrote :

Marked this bug fixed, and filed bug #1490907 to track the need for a unit test that verifies that the position is expressed in physical pixels.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.