webbrowser-app

Merge lp://qastaging/~fboucault/webbrowser-app/background_open_tabs_adjacent into lp://qastaging/webbrowser-app/staging

  1. background_open_tabs_adjacent
  2. Merge into staging
Proposed by Florian Boucault
Status: Merged
Merged at revision: 1609
Proposed branch: lp://qastaging/~fboucault/webbrowser-app/background_open_tabs_adjacent
Merge into: lp://qastaging/webbrowser-app/staging
Diff against target: 255 lines (+96/-15)
7 files modified
src/app/webbrowser/TabComponent.qml (+14/-11)
src/app/webbrowser/tabs-model.cpp (+9/-0)
src/app/webbrowser/tabs-model.h (+1/-0)
tests/autopilot/webbrowser_app/tests/__init__.py (+9/-0)
tests/autopilot/webbrowser_app/tests/test_contextmenu.py (+30/-4)
tests/autopilot/webbrowser_app/tests/test_tabs.py (+20/-0)
tests/unittests/tabs-model/tst_TabsModelTests.cpp (+13/-0)
To merge this branch: bzr merge lp://qastaging/~fboucault/webbrowser-app/background_open_tabs_adjacent
Reviewer Review Type Date Requested Status
Olivier Tilloy Approve
Review via email: mp+316226@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2017-02-01.

Commit message

Make new tabs opened in the background to be placed next to the tab opening requesting them instead of at the end of the list of tabs.

Description of the change

Make new tabs opened in the background to be placed next to the tab opening requesting them instead of at the end of the list of tabs.

To post a comment you must log in.
Revision history for this message
Olivier Tilloy (osomon) wrote : Posted in a previous version of this proposal

Can you please re-target the merge request to lp:webbrowser-app/staging?

The "Open link in new tab" and "Open link in new background tab" context menu entries should also open the new tab next to the caller (and there should be autopilot tests for those use cases).

Also, see one minor comment inline.

review: Needs Fixing
lp://qastaging/~fboucault/webbrowser-app/background_open_tabs_adjacent updated
1602. By Florian Boucault

Also apply opening new tab adjacent to current tab when using the following contextual actions:
- opening a link in a new tab
- opening a link in a new background tab
- opening an image in a new tab
- opening a video in a new tab

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

I am getting 17 failures out of 20 tests when running the test_contextmenu autopilot tests in narrow mode on desktop (gotta modify the initial window width to 50 GU in src/app/BrowserWindow.qml). This is a regression.

review: Needs Fixing
lp://qastaging/~fboucault/webbrowser-app/background_open_tabs_adjacent updated
1603. By Florian Boucault

TabsModel unit test: deallocate memory properly

1604. By Florian Boucault

Fix AP tests in narrow mode

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

I just observed that in chromium on desktop, when closing a tab that had been opened from another tab, the parent tab becomes current again, whereas in webbrowser-app the tab to the right of the one being closed becomes current. Maybe not something we want to address in this branch, but it would be a nice improvement to build on top of this branch.

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

webbrowser_app.tests.test_tabs.TestTabsManagement.test_ctrl_click_open_link_in_next_tab appears to be reliably failing in narrow mode.

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

> Maybe not something we want to address in this branch, but it would be
> a nice improvement to build on top of this branch.

Actually, I think it would make sense to implement this in this branch. I guess we need to add a property to BrowserTab so that it remembers its "parent" tab, and restores focus on it when closed.

Revision history for this message
Florian Boucault (fboucault) wrote :

> > Maybe not something we want to address in this branch, but it would be
> > a nice improvement to build on top of this branch.
>
> Actually, I think it would make sense to implement this in this branch. I
> guess we need to add a property to BrowserTab so that it remembers its
> "parent" tab, and restores focus on it when closed.

I think this behaviour could have been implemented before this branch. Therefore I would say it is very independent and think we should implement it separately.

lp://qastaging/~fboucault/webbrowser-app/background_open_tabs_adjacent updated
1605. By Florian Boucault

Merged staging

1606. By Florian Boucault

Fixed AP test test_ctrl_click_open_link_in_next_tab in narrow mode

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

LGTM, thanks.

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

> I think this behaviour could have been implemented before this branch.
> Therefore I would say it is very independent and think we should
> implement it separately.

Filed bug #1664990 to track its implementation.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches