Bug 22988 - [GTK] Need a public method to add a WebKitWebHistoryItem to WebKitWebBackForwardList.
Summary: [GTK] Need a public method to add a WebKitWebHistoryItem to WebKitWebBackForw...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Enhancement
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-12-24 22:03 PST by Hiroyuki Ikezoe
Modified: 2009-02-03 12:02 PST (History)
0 users

See Also:


Attachments
Proposed patch (2.59 KB, patch)
2008-12-24 22:07 PST, Hiroyuki Ikezoe
no flags Details | Formatted Diff | Diff
Revised patch (2.56 KB, patch)
2008-12-25 15:46 PST, Hiroyuki Ikezoe
no flags Details | Formatted Diff | Diff
Added test code. (7.02 KB, patch)
2009-01-12 18:17 PST, Hiroyuki Ikezoe
zecke: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hiroyuki Ikezoe 2008-12-24 22:03:40 PST
Nee a public method to manipulate history item in back-forward list for restoring session or something.
Comment 1 Hiroyuki Ikezoe 2008-12-24 22:07:19 PST
Created attachment 26245 [details]
Proposed patch

Wrap WebCore::BackForwardList::addItem.
Comment 2 Holger Freyther 2008-12-25 12:35:24 PST
Technically it looks sane, I assume the null check is not necessary though.

Could you explain why this method is needed? E.g. why do you need to inject items to the back forward list? What do you need to control? order...?

What you currently can do is, is create a WebKitHistoryItem (this will be added to the history) and then you should be able to navigate to this item? Do you try to do something else?
Comment 3 Hiroyuki Ikezoe 2008-12-25 14:59:09 PST
(In reply to comment #2)

> Could you explain why this method is needed? E.g. why do you need to inject
> items to the back forward list? What do you need to control? order...?

Example Restoring session

1) I read a web site.
2) And click a link on the web site, read it.
3) And go back the first page and I think that I read rest links on the page tomorrow.
4) Closing webkit.
5) Next day I can go on reading the rest links.

The important thing is that I can confirm the last link I read by forwarding the back forward list on the next day.
Comment 4 Hiroyuki Ikezoe 2008-12-25 15:46:11 PST
Created attachment 26248 [details]
Revised patch

Remove NULL check.
Comment 5 Holger Freyther 2008-12-26 06:51:14 PST
(In reply to comment #4)
> Created an attachment (id=26248) [review]
> Revised patch
> 
> Remove NULL check.

Cool. I fear I still need an explanation why this is needed? Could you try to explain that?
Comment 6 Holger Freyther 2009-01-07 16:40:06 PST
(In reply to comment #5)
> (In reply to comment #4)
> > Created an attachment (id=26248) [review] [review]
> > Revised patch
> > 
> > Remove NULL check.
> 
> Cool. I fear I still need an explanation why this is needed? Could you try to
> explain that?

Oops I missed the explanation. Looks like a good reason, I will take another look tomorrow.

Comment 7 Holger Freyther 2009-01-11 08:07:29 PST
Okay I think we want that. Could you come up with a test case (put it in WebKit/gtk/tests) defining the behavior of this new method? E.g. the impact on webkit_webview_can_go*, webkit_web_back_forward_list_get_current_item, webkit_web_back_forward_list_get_forward_item, webkit_web_back_forward_list_get_back_item? thanks
Comment 8 Hiroyuki Ikezoe 2009-01-12 18:17:20 PST
Created attachment 26657 [details]
Added test code.
Comment 9 Holger Freyther 2009-01-13 11:56:46 PST
Comment on attachment 26657 [details]
Added test code.

You might not set r=+ yourself, this is the second time you do it. Please pay more attention to that.
Comment 10 Hiroyuki Ikezoe 2009-01-13 19:04:39 PST
(In reply to comment #9)
> (From update of attachment 26657 [details] [review])
> You might not set r=+ yourself, this is the second time you do it. Please pay
> more attention to that.

I am sorry for my carelessness. I will keep my eye wide open next time.
Comment 11 Holger Freyther 2009-02-02 05:48:58 PST
Comment on attachment 26657 [details]
Added test code.

The patch looks good.
Comment 12 Holger Freyther 2009-02-03 12:02:43 PST
Landed in r40540.