[rekonq] Re: Review Request: Hybrid "new tab page"

Pierre Rossi pierre.rossi at gmail.com
Sun Apr 24 16:41:55 CEST 2011


> 
> Uhm... this patch is "monumental". And sincerely I don't like merging
> things like this. 

While I agree coming to the point where we have to do a big merge like this is sub-optimal, you have to keep in mind that trying out this new approach was experimental at first, so the development clearly couldn't have taken place in master from the start. When I started to see that this could work much better than what we currently have in place, I tried to get a bit more attention, but I think by then it was a bit late and this already seemed like my pet project to most.

In any case I am now convinced that this is the best way to go.

With WebKit2 around the corner, the future of most of the WebKit 1 synchronous APIs is yet to be settled, and while QWebElement is likely yo go away at one point, I think having a convincing use case for hybrid could help argue in favor of keeping this possible, but well this is not what's at stake here.

Anyhow, changes that are here don't really make sense on their own in my opinion, so I'm ok to split that up to make the review process easier (not before next week I'm afraid) but I don't really see the benefit of keeping only parts of it. If you find things that are not necessary I'd be more than happy to clean that up.
  
> First of all this is claimed to be the "new" hybrid
> home page. But in reality, this patch changes (in sparse order): - the
> "about" scheme handling (and more?) - a "new" scheme management (the
> "rekonq" one: to hide! Is this really needed?) 

the about handling changes (and the subsequent introduction of a rekonq scheme behind the scenes) are necessary so that data can be fed through the network access manager. The rationale between introducing a new scheme name behind the scenes is that about gets filtered by webkit (hardcoded, will only return a blank page without even going through the network access manager). To me this change doesn't make sense on its own (i.e if we're not going to use it).

> - some things in the
> bookmarks (not well identified) - some things in the history (not well
> identified) - introduces a new download manager 

All of these changes are intended to make it easy to expose all those objects to the script context. Again I don't feel they would make much of a difference on their own.

> - changes the way downloads history is managed and finally...
not sure what you mean by that. If you're alluding to the progressbars, well I'm sorry but I see no way of doing that with the old approach. If I actually changed the behavior, then feel free to consider it a bug and please tell me what's wrong about it so that I can fix it.

> - the new "hybrid" page (plus some fixes, changes not well identified
> through the commits messages).
> 
> So, if this is an ALL or NOTHING patch, I vote for NOTHING, sorry. I
> expect here the merges of the individual new features as standalones
> (eg: the download manager and the "scheme" handling change) and finally
> the "hybrid page" merge as "easy and NOT invasive" addiction. 
> 
> - Andrea
> 

As I said before I'll try to break that down but don't be surprised if individual changests don't bring anything to the table.

Cheers,
--
Pierre
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/rekonq/attachments/20110424/62470a35/attachment.htm 


More information about the rekonq mailing list