Bug 26452 - [Qt] Add ability to prepend an item to the history list
: [Qt] Add ability to prepend an item to the history list
Status: RESOLVED DUPLICATE of bug 30710
: WebKit
: 528+ (Nightly build)
: Other All
: P2 Normal
Assigned To:
: Qt
  Show dependency treegraph
Reported: 2009-06-16 11:50 PST by
Modified: 2009-11-24 07:48 PST (History)

Adds prependItem (3.72 KB, patch)
2009-06-16 12:02 PST, Adam Treat
no flags Review Patch | Details | Formatted Diff | Diff
updated patch with requested changes (3.62 KB, patch)
2009-06-16 15:39 PST, rick@lindenlab.com
eric: review-
Review Patch | Details | Formatted Diff | Diff
Updated WebKit patch for adding prepend API (3.93 KB, patch)
2009-06-22 13:32 PST, rick@lindenlab.com
no flags Review Patch | Details | Formatted Diff | Diff
minor cleanups + changelog (5.83 KB, patch)
2009-06-23 08:22 PST, Benjamin Meyer
manyoso: review-
Review Patch | Details | Formatted Diff | Diff


You need to log in before you can comment on or make changes to this bug.

Description From 2009-06-16 11:50:11 PST
Hi, and thanks.  I would like to be able to prepend items to the history list (i.e. put items in the browser's history list before the first one).  The reason is that I am using WebKit as a plug-in.  I want to shut down and restart WebKit, re-loading it with the history that I've kept track of.  To keep it simple, I propose adding a simple "prependItem" to QWebHistory, so that I can do this.
------- Comment #1 From 2009-06-16 12:02:04 PST -------
Created an attachment (id=31362) [details]
Adds prependItem
------- Comment #2 From 2009-06-16 12:05:09 PST -------
To be clear, the attached patch is from Rick Passeto, not me.
------- Comment #3 From 2009-06-16 15:33:36 PST -------
From an IRC chat with Kenneth: 
- change prpItem to newItem in both addItem() and prependItem()
- He also asked if ASSERT(newItem) was okay.  It looks like it is
- Also, in qwebhistory.cpp, he asked if WebCore should be namespaced in the file. 
------- Comment #4 From 2009-06-16 15:39:56 PST -------
Created an attachment (id=31386) [details]
updated patch with requested changes

This is an updated patch based on feedback from kenneth.
------- Comment #5 From 2009-06-18 18:20:36 PST -------
(From update of attachment 31386 [details])
ChangeLogs are missing.
------- Comment #6 From 2009-06-22 13:32:13 PST -------
Created an attachment (id=31670) [details]
Updated WebKit patch for adding prepend API

Another attempt at uploading the patch properly...
------- Comment #7 From 2009-06-23 08:22:28 PST -------
Created an attachment (id=31721) [details]
minor cleanups + changelog
------- Comment #8 From 2009-06-23 20:44:37 PST -------
The core code looks ok to me, but since the patch adds Qt API, a Qt person may want to review it.
------- Comment #9 From 2009-06-24 08:08:00 PST -------
I am ok with the API except for one thing I'd like to see addressed first:

1) What is wrong with storing the history like other clients do?  Arora also has need to shut down and re-open from time to time with an intact history list.  What is different about your situation as compared to Arora or other Qt clients that already do this with the existing API?
------- Comment #10 From 2009-06-24 08:23:49 PST -------
Ok, scratch my last question.  Arora and other clients *do not* restore history in this sense.  There is the QWebHistoryInterface::historyContains method, but that is only for visited links.  You are looking to make the back/forward list persistent across restarts.

However, I have another question... Can't you achieve the same thing with simple 'addItem'?  Why do you need to add 'prependItem' to WebCore?
------- Comment #11 From 2009-06-24 13:50:53 PST -------
The problem is that addItem() *appends* history items, so if there are already URLs in the history, they come after the current URL.

Even if this weren't so: in the case of re-initializing the history, if you call addItem(URL1), addItem(URL2), ... addItem(URLn),  the problem is that the "current URL" is wrong (it will be URL1).  There isn't API for "make this history item the current item" (there is a "goto this history item", but that's not the same.

Admittedly there are probably alternatives to prependItem, but this seemed simplest.
------- Comment #12 From 2009-06-24 14:21:36 PST -------
It seems to me your specific use case is to restore the history on restarting the plugin.  Given this, there should be no prior URLS in the back/forward list, right?

If true, then it seems you could simply append your items in reverse order, no?
------- Comment #13 From 2009-06-25 06:24:59 PST -------
(From update of attachment 31721 [details])
Take this out of the review queue as it seems things will need to be changed.  For instance, s/addItem/prependItem/ to QWebHistory and also s/const QString &urlString/const QUrl &url/ etc, etc.
------- Comment #14 From 2009-10-29 04:32:50 PST -------
Functionality of saving and restoring the QWebHistory should be in Qt 4.6.

The bug is similar to bug 30710. I suppose, this one may be closed as duplicated. Please, verify if am I correct.
------- Comment #15 From 2009-11-24 07:48:06 PST -------
I'm closing as duplicate of bug 30710.

*** This bug has been marked as a duplicate of bug 30710 ***