Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Go parent folder" feature only as a WebExtension/Firefox core feature #1

Open
NicolasWeb opened this issue Feb 5, 2018 · 19 comments
Labels
external dependency Requires a feature outside of BSP2 to be made available

Comments

@NicolasWeb
Copy link

Thank you for translating "Sidebar bookmark search plus" XUL extension (and its accompanying "Show parent folder" and "Go parent folder" ones) to WebExtension ! :-)

I would like to let you know about :

  1. go-parent-folder thread on discourse https://discourse.mozilla.org/t/new-to-extension-development-bookmark-api-and-search-function/21686
  2. Bug 1272891 to implement go-parent-folder addon as a Firefox feature. Comment 6 is a good good new. https://bugzil.la/1272891
  3. Bug 1420247 to migrate go-parent-folder addon to webextensions. You might be helpful with this bug dependencies too. https://bugzil.la/1420247

Do you think with the work you've done on Bookmark-search-plus-2 you could help one/both of these bugs to go forward/fixed please ? Or even implement a patch from the source code of your addon ?

I would love to be able to show parent folders while not having to open the web page of search result bookmark.

@aaFn
Copy link
Owner

aaFn commented Feb 5, 2018

Hello @NicolasWeb, thank you much for the information.

Hehe, ironic if after 17 years of asking, we converge to a solution a few months after I made the effort (1.5 month of week end work basically), but it was fun doing that anyway, and I am ready to help wherever I can ... inside or outside the engine as needed.

Yes, I believe some of the code I wrote could be reused inside the engine itself .. I tried to be as similar to the product behavior, so in my strive to do so, I believe I made some choices in the objects used which should be close to what is used in the main code, and therefore make way for some reuse or acceleration.
As for the "algorithms" I used (sorry no other word, don't want to be pedantic), they should be quite independent, so as long as the data structure to make them work is there .. or not far .. should be an easy thing.

To be honest, this presumed easiness would be a side effect though, as the main reason why I tried (within limits of Web Extensions ..) to be as similar to the old "Sidebar bookmark search plus" and to the normal Bookmarks Sidebar is to be as less intrusive as possible, and as less habit changing as possible, for people like me who have the bookmarks sidebar open on their left all navigation time long, and search for bookmarks, reorganize, drag, drop tabs and links to new bookmarks in the same folder as their brothers and already existing ones ..

"Bookmark search plus" was doing a very great job at that: thanks to XUL, it was a basic additional layer on top of the normal code as I get it .. therefore leaving the original behavior intact and adding what was needed.
In particular, the idea of reducing the search pane to show again the bookmark tree below it, and to visually relate the search results with their position in the bookmark tree itself was in my opinion a genius trait from Alice0775 .. so simple and intuitive :-)

This is really why I tried to stick to that as close as possible, and if as a side effect this makes my code useful to truly solve the situation inside the engine other than by adding an extension, count me in to help.

Cheers.

@JohnLukeBentley
Copy link

@aaFn once again, it is great work you've done on "Bookmark search plus 2". As is it gives us most of the Go Parent Folder functionality that has been sorely missed. So it is useable out of the gate. Thanks for putting in those 1.5 months!

It's already been agreed but, just to add another voice, I do think if the code you've written could be incorporated into the Firefox (FF) core: that'd probably be for the best. ( I note this is an easy view to take when one (i.e. me) doesn't have to do the work. :) )

For one thing when comparing the loading of the bookmark tree in the native "Bookmarks" manager Vs "Bookmark search plus 2", the former loads (in effect) instantly while the later loads with some delay (with my set of bookmarks about 1 second). No doubt the shortness of Bookmark search plus 2 delay has been hard won against various limitations around accessing the (so I'm now informed) sqlite DB. But presumably if you could work with the FF core code, you'll be able to leverage their native access to whatever form those bookmarks are stored and loaded.

Perhaps, given your "count me in", you have to make a further decision about where you want to direct your efforts:

  • Abandon effort on "Bookmark search plus 2" and start incorporating that code into the core; or
  • Both improve "Bookmark search plus 2" and start incorporating that code into the core;

If it is the later then @NicolasWeb raises the first issue that arose when I started using the extension

I would love to be able to show parent folders while not having to open the web page of search result bookmark.

I'd agree with that. Or perhaps some kind of option to show the parent folder(s) either: with opening the webpage; or without opening the wepage. E.g. Click for one action; Shift+Click for the other. Certainly when using the old Go Parent Folder I frequently searched for one bookmark in order to locate another (e.g. that I know is nearby in terms of folder grouping) - and in that case the searched for bookmark I therefore don't want to load.

But that issue is separate to the issue of whether the extension should incorporated into the core. So, I suppose, if you let us know whether you want to also continue to improve "Bookmark search plus 2" I'd post it as a separate issue - if you answered in the affirmative.

And, I'd like to emphasize, I don't think it would be bad choice to abandon effort on "Bookmark search plus 2" if you wanted to incorporate that code into the core.

@aaFn
Copy link
Owner

aaFn commented Feb 7, 2018

@JohnLukeBentley, so now I believe you made me understand that sentence from @NicolasWeb which I was not sure to interpret properly .. thank you much :-D

I would love to be able to show parent folders while not having to open the web page of search result bookmark.

Sorry to be so slow gents ... as a matter of fact, you can do it :-)

Simply, click on the search result on left or right of the "image+text", where the mouse pointer is an arrow, and not where it is a "hand with finger" ... and you'll get what you want.
Indeed, you'll notice that when you hover the mouse over the result you want to look for, the whole result row is highlighted .. that is to signal to you that you can click anywhere on it and:

  • left clicking on a part which is not the link nor the image will simply show where it is in the bookmark pane, no other action,
  • left clicking on the link or image will open it in current tab, and also show the item in the bookmark pane
  • right clicking anywhere in the result will show a context menu to chose where to open the link -> I guess I could add a menu option to "show bookmark" also, to make it "not only one way of doing things" .. let me know.

By the way, the same behavior occurs for normal bookmarks, clicking outside the link/image simply highlights it with no action, clicking on it opens it in current tab, and right click = context menu ..

On the second point, about delay before showing .. Yes I started to work in the add-on on an option to immediately show the tree while it is building, asynchronously loading it. I didn't really make it good for now as it was experimental, so you see no difference if you activate it in the add-on options panel (although it really is asynchronous behind the scene).

Now, a dialog with a guy who has 80000 bookmarks (I am a micro-small player besides him with my 1800 bookmarks lol) on the add-on review page, made me envisage a number of alternatives to really make that effective. I'll try to work out something by end of next week that should visually get more responsive .. although that won't really improve real loading perfs by themselves, just visual feeling.
Now, it is clear I won't be able to reach the load performances of native code .. no dreaming, but working on the visual interactivy + maybe on a few optimi(s/z)ations, I might be able to do something effective .. let's see.

As for the last question. I'll be practical =

  • given I don't know if and how fast I can get co-opted by the FF community, and my contributions to be accepted
  • given I don't know the source code and my easiness assumptions above could be presumptions, and so it could take me some time to really absorb the core code and to propose an effective contribution

I'd vote for #2 = continue BSP2, and at the same time contribute to core if there is community acceptance for me to do that. And when core is ready and there is no more value in BSP2, discontinue it.
At least this is where I would stand for now :-)

Thank you much for your both detailed feedback, and let me know what in the above possible improvements you would vow for, or any suggestion.

@JohnLukeBentley
Copy link

I'd vote for [...]2 = continue BSP2, and at the same time contribute to code if there is acceptance for me to do that. And when core is ready and there is no more value in BSP2, discontinue it.
At least this is where I would stand for now :-)

That sounds like the right plan :)

Therefore I've opened up new threads on the remaining two issues:

@NicolasWeb
Copy link
Author

NicolasWeb commented Mar 26, 2018

The bug 1420246 I opened on bugzilla to improve WebExtensions API and help go-parent-folder addons like migration will be discussed on March 27, 2018 meeting.

I'm not sure to be able to join the meeting, and btw, I think that @aaFn have better skills than me to discuss that.
@aaFn would you be able to join this meeting ?

(All needed info is in comment 1 to join the meeting)

@aaFn
Copy link
Owner

aaFn commented Mar 27, 2018

Hello @NicolasWeb , unfortunately I am currently in business travel in NY, given the schedule I understand that will be tomorrow around 13:55 to 14:00 NY time, and I will not be available at that moment. Or let me know if I understood wrong.

@NicolasWeb
Copy link
Author

Hi @aaFn, the order in which the bugs are checked might not be the one listed in the doc. May you be available at an other time from 13:00-14:00 ?

The root bug of that one is bug 1420247, that list all API changes needed to port go-parent-folder to WebExtension.
With your experience on BSP2 and the actual API list, for the basic Show bookmark feature in the native sidebar:
-1/ do you agree that's all what we need ?
-2/ do you agree that the actual API does not work for our needs ?
-3/ do you have some argues (please, do ; I'm not a dev...) that need to be eared and I can expose during the meeting / Or one of us write on bug 140246 ?
-4/ do you have some feedback from writing BSP2 to give that expose that even if BSP2 already exists as go-parent-folder=like WebExtension, we need new API ? (to me it's you had to rewrite a whole sidebar because of the actual API abilities)

I originally opened that bug because of this discourse thread.

@aaFn
Copy link
Owner

aaFn commented Mar 27, 2018

@NicolasWeb , thanks for the history, this is useful.
I believe I would be able to join (at least by phone) between 13:00 and 13/30 NY time, however the schedule says it should start at 10:30 Pacific time, which means 13:30 NY time .. or is there a previous 1/2 hour session before that whch the bug could be transferred to ?

In terms of comments and contents, I can tell that bookmarks.show() represents some amount of code, but this is probably one of the best way to provide the functionality to extensions, and worth the investment given the big lack this represents to users (this is why I decided to go through the hassle of creating a new UI, mimicking as much the real one ..).

However:

  • it requires the native sidebar to be open
  • I am not sure how it would work in Android .. there is no sidebar as I can see it, and this is why I dropped support for Android when publishing my add-on
  • It won't work on the library since as I get it no WebExtension is working in the library

Therefore a WebExtension using it would need to be a popup, not a sidebar. Or something in the web page itself, but this would be quite limited I presume.

It the menus.ContextType feature can be made to work from the library and from the bookmark sidebar (https://bugzilla.mozilla.org/show_bug.cgi?id=1419195), in particular from the search pane, that would enable the above bug to work from inside the native sidebar ... with the additional complexity that then the native sidebar should be able to show the bookmark pane at the same time than the search pane (which is the option I chose in BSP2, in line with the initial BSP extension from alice0775).

Hope all the above makes sense :-) I am adding it to the bug.
Let me know if there is a chance that this can be discussed between 13:00 and 13:30 NY time.

@NicolasWeb
Copy link
Author

Ooops, my mistake, sorry.
If we start the meeting with that bug at 13:30, will you be able to join ?
I can ask caitlin :)

Thanks for the long answer :)
The way to implement the boookmarks.show() function need to be discussed. But I hope that working on Firefox core code would help simplifying the tree troubles you faced while doing the WebExtension. We might even propose to split it in main/following bugs to implement the whole complex feature you described (I do think that being able to show the bookmark pane at the same time than the search pane could be a following bug) .
BTW, this "Show bookmark" feature exist in Chromium, so that could be useful to check they implementation.

@aaFn
Copy link
Owner

aaFn commented Mar 27, 2018

Indeed, I believe there should be companion features .. I added some more on the bug itself to open the discussion https://bugzilla.mozilla.org/show_bug.cgi?id=1420246#c3

I will try to join at 13:30 NY time for 5 minutes, by phone.
Meanwhile I have set myself on the #addons IRC channel to follow up

@NicolasWeb
Copy link
Author

Caitlin agreed to move this bug as the first one for this meeting.

I don't remember that much activity on the IRC channel during the meeting (but I may be wrong). If you can, you should definitely join by voice (so phone...)

@aaFn
Copy link
Owner

aaFn commented Mar 27, 2018

Ok, thanks @NicolasWeb , I will join by phone, just monitoring the IRC channel for keeping abreast

@NicolasWeb
Copy link
Author

Just sent a comment on the API bug, to try to say what my broken microphone didn't allow me to say during the meeting ;)

@aaFn
Copy link
Owner

aaFn commented Mar 28, 2018

I read it, well said. Thank you for the summary, this is a very good one, and well put.

@rocketaz rocketaz mentioned this issue Jul 19, 2018
@aaFn aaFn added the external dependency Requires a feature outside of BSP2 to be made available label Aug 11, 2018
@keunes
Copy link

keunes commented Jul 7, 2019

I'd be very much interested in this. Do you expect still any developments on Firefox' side?

@aaFn
Copy link
Owner

aaFn commented Jul 7, 2019

Hello @keunes , for now the feature request (bug) is still unassigned, so it should happen one day, but no idea of when at this stage.

@Romko-M
Copy link

Romko-M commented May 3, 2020

Bookmark search plus 2 or Show Parent Folder SeaMonkey 2.53 extension is badly needed!

@aaFn
Copy link
Owner

aaFn commented May 3, 2020

Hello @Romko-M, well for now I only do Firefox extensions.
I guess I will extend to similar browsers one day.
.. until Firefox and these other browsers add the feature natively themselves, since best would be that they add it, and then I would stop BSP2 😀

@mzso
Copy link

mzso commented Nov 22, 2020

Another few years an Mozilla might actually get to it...

@VLM-TechEd VLM-TechEd mentioned this issue Apr 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external dependency Requires a feature outside of BSP2 to be made available
Projects
None yet
Development

No branches or pull requests

6 participants