Bug 22988 – [GTK] Need a public method to add a WebKitWebHistoryItem to WebKitWebBackForwardList.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Hiroyuki Ikezoe]

Nee a public method to manipulate history item in back-forward list for restoring session or something.

[Hiroyuki Ikezoe]

Created attachment 26245 [details]
Proposed patch

Wrap WebCore::BackForwardList::addItem.

[Holger Freyther]

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?

[Hiroyuki Ikezoe]

(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.

[Hiroyuki Ikezoe]

Created attachment 26248 [details]
Revised patch

Remove NULL check.

[Holger Freyther]

(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?

[Holger Freyther]

(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.

[Holger Freyther]

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

[Hiroyuki Ikezoe]

Created attachment 26657 [details]
Added test code.

[Holger Freyther]

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.

[Hiroyuki Ikezoe]

(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.

[Holger Freyther]

Comment on attachment 26657 [details]
Added test code.

The patch looks good.

[Holger Freyther]

Landed in r40540.