[Merge] lp:~uriboni/webbrowser-app/ctrl-click-link into lp:webbrowser-app
Olivier Tilloy
olivier.tilloy at canonical.com
Thu Nov 5 10:31:50 UTC 2015
Review: Needs Fixing
The analysis of the cause of the issue is good, but there is a much simpler way to fix it.
Nothing outside of BrowserTab should modify the visibility of webviews, as this would break the internal state that BrowserTab maintains. Instead, simply defaulting the 'visible' property of BrowserTab to false (like the 'current' property) is enough to fix the problem, because the tab will be created invisible, so the webview it contains will be invisible too.
As for the new autopilot test, there are two issues with it:
- Testing that a user action doesn’t trigger a response is impossible (you can never guarantee that the response won’t be triggered after a delay). Instead we should always try and test the reverse scenario, where a user action eventually triggers a response (in that case a click triggers a navigation).
- When triggering the bug (without the fix), the new webview is overlaid on top of the current one, but mouse events go to the current webview, not the new one (yeah, that’s weird). So we can’t rely on that for the test. We might be able to use https://developer.ubuntu.com/api/autopilot/python/1.5.0/autopilot.display.Display/#autopilot.display.get_screenshot_data with e.g. the new page opened in a background tab having a red background (note that I have never used that method, not sure if/how it works).
--
https://code.launchpad.net/~uriboni/webbrowser-app/ctrl-click-link/+merge/276640
Your team Ubuntu Phablet Team is subscribed to branch lp:webbrowser-app.
More information about the Ubuntu-reviews
mailing list