[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).


Other related posts: