[haiku-webkit-commits] Re: r315 - in webkit/trunk/WebKit: . haiku/WebPositive haiku/WebPositive/support
- From: Maxime Simon <simon.maxime@xxxxxxxxx>
- To: haiku-webkit-commits@xxxxxxxxxxxxx
- Date: Tue, 16 Mar 2010 19:22:00 +0100
On Tue, Mar 16, 2010 at 19:08, Stephan Assmus <superstippi@xxxxxx> wrote:
>
> On 2010-03-16 at 18:08:00 [+0100], Ryan Leavengood <leavengood@xxxxxxxxx>
> wrote:
> > On Tue, Mar 16, 2010 at 9:49 AM, <webkit@xxxxxxxxxxxxxxx> wrote:
> > > Author: stippi
> > >
> > > * Wire everything to complete the bookmark support.
> >
> > I suppose it is too late now for you to use this, but I'm pretty sure
> > I mentioned it before:
> >
> > http://github.com/leavengood/Haiku-Browser/tree/master/libbookmark/
>
> You didn't mention the link the first time, unfortunately. I meant to ask
> for it, but then I realized again that bookmarks in Haiku are just plain
> files with a META:url string attribute.
>
> > Even if the code is not that useful I think it is still smart to have
> > a libbookmark to encapsulate some of the bookmark management.
>
> The code I wrote is not very extensive. I've looked over the code Maxim
> wrote. I am sorry to say that it is full of memory leaks (perhaps Maxim has
> been influenced too much by Java programming?), and doesn't use the
> standard bookmark attributes (uses "URL" and "Title" instead of "META:url"
> and "META:title"), so it wouldn't be compatible with existing apps and
> bookmarks. Moreover I think my solution with the lazy evaluating BNavMenu
> from Tracker is more elegant and re-uses existing code. Maxim's code
> implements populating a menu, but it may have long delays for large
> bookmark collections. The actual handling of bookmarks, in terms of looking
> which ones are already there to avoid duplicates, and what to do in case of
> collisions, isn't implemented in Maxim's code, but is implemented in what I
> wrote today.
>
>
That's the price of 5 years of academic projects in Java. :-(
But if I go over this code (in a time) I could repair those things.
I has not aware of BNavMenu, but in fact it seems more efficient
than my code.
I do see the value in supporting a global bookmark folder, however, so I
> may refactor the code so that it can be provided as library.
>
> I hope what I wrote above doesn't come across the wrong way, I don't mean
> to discredit Maxim's work, but I would have had to rewrite a lot of it. At
> least in that regard, I didn't waste working hours.
>
>
And you did well! I wrote that code at the beginning of the summer with
a limited knowledge of both BeOS API and C++ (not that limited for C++
but enough to create some memory leaks).
> > As for the discussion over what to do when clicking a folder of
> > bookmarks, I agree with Hanna that it could be bad if there are a lot
> > of bookmarks and the browser tried to open all of them in tabs. I
> > think it might be better to make it explicit and just add a menu item
> > like in Firefox for "Open All in Tabs" (at the end of the submenu,
> > after a separator.) Clicking of the menu item itself can work like it
> > does now, opening it in Tracker.
>
> I already commited a change which does the asking when there are more than
> 10 bookmarks to be opened at once. I can see the value of "Open all in
> tabs", but it needs to be at the top of the menu, not at the bottom, so it
> is always reachable and not burried at the end of a very long list.
>
> Best regards,
> -Stephan
>
>
I don't know for others but I would really appreciate to have a "Open all
in tabs" button. I haven't tested your change yet but it may feel strange
when experimenting it the first time (for unaware people I mean).
Regards,
--
Maxime
Other related posts:
