webbrowser-app

Merge lp://staging/~osomon/webbrowser-app/historyUpdateOnLoadCommitted into lp://staging/webbrowser-app

historyUpdateOnLoadCommitted
Merge into trunk

Proposed by Olivier Tilloy on 2015-05-18

Status:

Merged

Approved by:

Olivier Tilloy on 2016-03-04

Approved revision:

1029

Merged at revision:

1372

Proposed branch:

lp://staging/~osomon/webbrowser-app/historyUpdateOnLoadCommitted

Merge into:

lp://staging/webbrowser-app

Diff against target:

391 lines (+229/-40)

6 files modified

src/app/webbrowser/Browser.qml (+29/-2)
src/app/webbrowser/history-model.cpp (+37/-1)
src/app/webbrowser/history-model.h (+2/-1)
tests/autopilot/webbrowser_app/tests/http_server.py (+32/-1)
tests/autopilot/webbrowser_app/tests/test_history.py (+80/-34)
tests/unittests/history-model/tst_HistoryModelTests.cpp (+49/-1)

To merge this branch:

bzr merge lp://staging/~osomon/webbrowser-app/historyUpdateOnLoadCommitted

Related bugs:

Bug #1455858: History AWOL with YouTube "get more videos" button.	Medium	Invalid
Bug #1549780: [regression] favicons not stored in the history database	High	Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Riccardo Padovani (community)			Approve on 2016-03-01
PS Jenkins bot	continuous-integration		Needs Fixing on 2016-03-01
Ubuntu Phablet Team		2015-05-18	Pending
Review via email: mp+259361@code.staging.launchpad.net

Commit message

Store entries in the history database on load committed, not load succeeded.
This ensures that content-initiated navigations are also stored.

Revision history for this message

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

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-05-23:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Riccardo Padovani (rpadovani) wrote on 2015-06-11:

+ browser.historyModel.add(webviewInternal.storedUrl, webviewInternal.storedTitle, icon)

Why this and not

browser.historyModel.add(webviewInternal.storedUrl, title, icon)

I mean, we're updating the database, it's better to do it with last data we have, isn't it?

review: Needs Information

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-06-11:

> + browser.historyModel.add(webviewInternal.storedUrl,
> webviewInternal.storedTitle, icon)
>
> Why this and not
>
> browser.historyModel.add(webviewInternal.storedUrl, title, icon)
>
> ?
>
> I mean, we're updating the database, it's better to do it with last data we
> have, isn't it?

Some pages dynamically update the title to convey live information (like the ugly "scrolling title" effect, see e.g. http://htmlmarquee.com/title.html). In that case we don’t want to update the history DB everytime the title changes. And if the title has changed when we get an icon, we don’t want to update it either, because it’s most likely a dynamic change.
This is consistent with how chromium on desktop behaves.

Revision history for this message

Riccardo Padovani (rpadovani) wrote on 2015-06-11:

Looks good to me now :-)

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-06-11:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1879/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/2084/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/2085/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/2092/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/2096/rebuild

review: Needs Fixing (continuous-integration)

lp://staging/~osomon/webbrowser-app/historyUpdateOnLoadCommitted updated on 2016-02-26

1021. By Olivier Tilloy on 2016-02-26: Merge the latest changes from trunk and resolve conflicts.
1022. By Olivier Tilloy on 2016-02-26: Add an update(…) method to HistoryModel, to allow updating the title and icon of an entry without incrementing the corresponding number of visits.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-26:

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

review: Needs Fixing (continuous-integration)

lp://staging/~osomon/webbrowser-app/historyUpdateOnLoadCommitted updated on 2016-02-26

1023. By Olivier Tilloy on 2016-02-26: Simplify code a bit.
Do not prevent dynamic favicon updates from updating the history database, to be consistent with what chromium does.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-26:

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

review: Needs Fixing (continuous-integration)

lp://staging/~osomon/webbrowser-app/historyUpdateOnLoadCommitted updated on 2016-02-29

1024. By Olivier Tilloy on 2016-02-29: Add some autopilot tests to verify that favicons are stored and updated in the history database.
1025. By Olivier Tilloy on 2016-02-29: Factor out common code.
1026. By Olivier Tilloy on 2016-02-29: Add autopilot tests to verify that a page’s title is stored in history, but not dynamically updated.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-29:

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

review: Needs Fixing (continuous-integration)

lp://staging/~osomon/webbrowser-app/historyUpdateOnLoadCommitted updated on 2016-02-29

1027. By Olivier Tilloy on 2016-02-29: Add an autopilot test to verify that history.pushState() inserts a new entry in the history database.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-29:

FAILED: Continuous integration, rev:1026
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2647/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/6678
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1400
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1400/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1400
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-touch/5068
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/6689
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/6689/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/27945

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-29:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Riccardo Padovani (rpadovani) wrote on 2016-03-01:

I only read the code and seems good to me, just a couple of very little improvements inline.

I'll test asap.

review: Needs Fixing

lp://staging/~osomon/webbrowser-app/historyUpdateOnLoadCommitted updated on 2016-03-01

1028. By Olivier Tilloy on 2016-03-01: Swap the order in which conditions are evaluated, from cheapest to most expensive.
1029. By Olivier Tilloy on 2016-03-01: Move code out of else block to avoid compiler warning (control reaches end of non-void function [-Wreturn-type]).

Revision history for this message

Olivier Tilloy (osomon) wrote on 2016-03-01:

Thanks, those were very good points, I addressed them.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-03-01:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Riccardo Padovani (rpadovani) wrote on 2016-03-01:

Thanks! Tested a bit and seems to work like a charm.

review: Approve

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

to all changes:

Olivier Tilloy

Ubuntu Phablet Team

to status/vote changes:

Rachel Liu

webbrowser-app

Merge lp://staging/~osomon/webbrowser-app/historyUpdateOnLoadCommitted into lp://staging/webbrowser-app

Commit message

Description of the change

Preview Diff

Subscribers