Switch browsing history away from QWebHistoryInterface and add titles.#1350
Switch browsing history away from QWebHistoryInterface and add titles.#1350The-Compiler merged 4 commits intoqutebrowser:masterfrom
Conversation
toofar
force-pushed
the
history-title
branch
2 times, most recently
from
9643181 to
3d48895
Compare
|
I decided to not add this to v0.6.0 - it looks good and much simpler than I thought it'd be, but as you said, there's potential for subtle issues. That's why I'd rather have it cook in -git a bit before releasing a version with it 😉 |
| """ | ||
| history = WebHistory(parent) | ||
| objreg.register('web-history', history) | ||
| QWebHistoryInterface.setDefaultInterface(history) |
There was a problem hiding this comment.
This will still be needed to color visited links. I suggest you re-add this, simply ignore what's happening in addHistoryEntry, and add another method (say add_url or so).
|
I added the call to setDefaultInterface back and that works. Though with the whole not saving redirects thing sites, like search engine result pages, that link via redirectors don't show their links as visited. Maybe it would be best to save both urls clicked on and pages visited. Bit more noise in the history file but that isn't an issue. Just need to control for duplicates... |
| # QUrl.UrlFormattingOption. | ||
| if not self._orig_url.matches(self.cur_url, | ||
| getattr(QUrl, 'None', | ||
| getattr(QUrl, 'None_'))): |
There was a problem hiding this comment.
This is a bit unfortunate indeed... But QUrl.UrlFormattingOption(0) seems to work as well and seems cleaner.
Adds a title to the HistoryEntry class and includes it in the serialization stuff. Not currently set from anywhere. Not sure if anything more needs to be done to support non-ascii characters. Everything works fine for me with unicode chars in url and title but everything in my stack is utf-8.
Now adds a url to browser history once we have connected and got enough data to start rendering the page. The previous approach saved urls as soon as navigation was initiated, so upon encountering a redirect the final url wasn't saved. Using layout started rather than load finished means that pages whose contents manage to load minus one troublesome asset will still be saved.
This allows webkit to color links that are clicked on but never rendered as visited too. It also means if you get redirected from eg http://site.com to http://site.com/ you have essentially duplicates in your history. This makes the history completion a bit noisier. I suppose normalising paths before checking for duplicates might help. Also note that otter has an isTypedIn flag which might be used for dealing with this.
|
I'd like to get this in before I release v0.7 this week and start to refactoring everything for QtWebEngine. If this is okay for you I'll just take over and add some tests myself. |
| QUrl.UrlFormattingOption(0)): | ||
| # If the url of the page is different than the url of the link | ||
| # originally clicked, save them both. | ||
| history.add_url(self._orig_url.toDisplayString()) |
There was a problem hiding this comment.
Is there a reason you're not adding the title here, or is that just an oversight?
|
I'm adding a few comments about things I find out - you can ignore them, I'll fix it up as I'm writing tests anyways. |
| # If the url of the page is different than the url of the link | ||
| # originally clicked, save them both. | ||
| history.add_url(self._orig_url.toDisplayString()) | ||
| history.add_url(self.cur_url.toDisplayString(), self.title()) |
There was a problem hiding this comment.
toDisplayString has the problem of e.g. decoding spaces in the URL which is undesirable here. This should probably be toString(QUrl.FullyEncoded | QUrl.RemovePassword) instead.
|
And merged! Some fixes on top of it: |
3 participants
Changes history items to be added when initialLayoutCompleted is fired as opposed to when navigation is initiated. Regarding #1345 this method saves the final url after redirects and doesn't save the url of the redirect.
There are some things I am unsure about:
Here is some comparisons with dwb and otter:
Otter (for the qtwebkit backend at least) calls handleHistory() at a few
places but calls after the first one update an entry instead of adding a new
one. It gets called on title changed, navigate (load started I think) and load
finished. Due to events that would update current url and title not firing for
redirect urls otter saves the originally requested url as url and title into
history so if there is a chain of redirects all but the first will be saved
wrong.
dwb and QupZilla use load finished. They only save the final page on
redirects.
This method, the old method, dwb and otter all save the requested url to
history on DNS and 404 errors.
This change is