hnakamur / FormatLink-Firefox

A Firefox web-extension to copy the URL and the title or the selected text to clipboard
https://addons.mozilla.org/ja/firefox/addon/format-link3/
MIT License
45 stars 8 forks source link

New Preferences #9

Closed jflorian closed 7 years ago

jflorian commented 7 years ago

Thanks for this WebExtension. As a longtime user of CoLT, I needed something for this new era.

Would it be possible to have a preference to change to which buffer the link is copied? On Linux, we have 3: XA_PRIMARY, XA_SECONDARY and XA_CLIPBOARD. XA_SECONDARY isn't really used anymore, but the other two are very common. Right now FormatLink is using XA_CLIPBOARD, but I'd prefer it to use XA_PRIMARY so that I need only middle-click to paste.

I'd also like the "default" format to not require my choosing of the desired format. When I right-click a link I have to choose "default" or whatever it points to, which seems an unnecessary extra step.

hnakamur commented 7 years ago

Thanks for your comment.

I'm afraid it's not possible to have a preference to change the buffer. WebExtension APIs are very limited due to security reason. Copying to clipboard is implemented as emulating the "copy" menu command: https://github.com/hnakamur/FormatLink-Firefox/blob/fd443319cb940d6bbe483b200162080e814d92e2/clipboard-helper.js#L16

I understand choosing "default" is an unnecessary extra step. However I'd like to use other formats too.

Context menus for a WebExtension is structured automatically. If you have only one menu item, the item is on the toplevel context menu, so an extra step is not needed. However if you have multiple menu items, the menu group for the WebExtension is automatically created at the top level.

I decided I go for latter since I want to choose formats. You can fork this extension and modify so that it has only one menu item to suit your need.

jflorian commented 7 years ago

I too will, at times, most definitely want the other formats (and the templating setup is great BTW). Could there simply be two options on the context menu: "default format" and "choose a format" with the latter essentially doing what's available now?

hnakamur commented 7 years ago

I am lazy to implement the UI for choosing a format separately. Instead, if you choose an menu item like "as Markdown" with control clicked, then it becomes the new default.

I'm using this feature when I want to change the format, and use "as Default" menu for other times.

Oops, it didn't work and I just fixed now. Fix setting default format with context menu with ctrl down by hnakamur · Pull Request #10 · hnakamur/FormatLink-Firefox I submitted the fixed extension as version 1.3.5 for review at addons.mozilla.org. Wait for 1.3.5 to be approved or you can use the extension debugger to use 1.3.5 right now.

To tell the truth, actually I'm not using context menus of this extension that much. I had created the Format Link for Chrome a long ago and it has only the toolbar button until recently (since I was lazy to implement context menus). So I am using the toolbar button most of the time.

The distance you need to move the mouse cursor to the toolbar is longer than context menu, but you need less clicks than the context menu. You can copy the url as default format with 1 click, just clicking the toolbar button. You can copy the url as desired format with 2 clicks, clicking the toolbar button then the radio button. You can copy the url as desired format and make that default with 3 clicks, clicking the toolbar button, then the radio button, and then "Set as default" button.

hnakamur commented 7 years ago

Ah, but the targets for the toolbar button and the context menu are different. The toolbar button copies the URL for the current page, The context menu copies the URL for the selected link.

I'm becoming inclined to change to just one context menu which uses the default format. To change the format, you need to use the toolbar button and the "Set as default" button. I'll think about this for a day or two before modifying the context menus.

jflorian commented 7 years ago

I'm becoming inclined to change to just one context menu which uses the default format. To change the format, you need to use the toolbar button and the "Set as default" button.

That seems like an excellent model. Personally, I tend not to use the toolbars at all unless I want to diddle the preferences somehow. Then again, I also use various VIM plugins (VimFx currently), so I'm already disqualified as a typical user.

hnakamur commented 7 years ago

I've created the pull request for this change. Just leave the context menu for the default format and delete other items by hnakamur · Pull Request #11 · hnakamur/FormatLink-Firefox

I'll think about this for a day or two before merging this pull request.

hnakamur commented 7 years ago

@IzzySoft Could you tell me what do you think about the above change? It deletes the context menus for non-default formats, and the menu item for the default format goes to one level up (toplevel) in the context menu.

IzzySoft commented 7 years ago

@hnakamur I've a bit mixed opinions on that. First, like the two of you, in ~99% of all cases I use the same format anyway (Markdown) – and if I wanted just the URL or the Title, there are separate top-level menus for that available (not sure if FF provides those itself or it's one of the other Addons I use). But then, if one needs a different format (or other users need to switch frequently), that should be easy and intuitive (there are e.g. times where I frequently need to toggle between "Wikipedia" and "Markdown"). So:

Not sure how crippled limited WebExt is, but could that be made configurable somehow? Ideal case would be to have the default format at top-level, and the "sub-menu" in addition (with the option to toggle the one or the other off) – if that's possible at all.

I currently still have CoLT in use additionally to FormatLink (the former for the places FL doesn't yet cover – you know what I mean), so no problem if your merge turns out in a way confusing me and we fix that later :wink:

@jflorian As you see, I'm also coming from CoLT. A pity that Jonah stops his addons, understandable as it is. Be welcome to my status list in case you're still wondering about other addons; feedback welcome :wink:

jflorian commented 7 years ago

Agree completely with @izzysoft here. I have no context menu option for just the title (with CoLT disabled) so must be some other addon. I did create a new FormatLink entry called CoLT :) that is just "{{title }}" as that is a primary use case for me.

jflorian commented 7 years ago

Also I would try a dev version here but know nothing about installing add-ons outside of AMO. I develop in a different world.

hnakamur commented 7 years ago

@jflorian @IzzySoft Thanks for your comments! I agree changing the default format should be easy and intuitive. However I prefer the easy side, I mean the less steps in operation to change formats.

As of now, you can change the default format in the settings page of this extension. However I feel going to the settings page needs too many steps. As for a user myself, I use three formats, Markdown, reST, and text, and change the default format rather frequently.

When I created the first version of this extension, I thought about adding the "settings" button or link to the pop up page which is opened by clicking the toolbar button, but I prefer less steps to change the format, so I settled to the "Set as default" button as it is now.

As for menu items, it's not possible to have both a top-level menu item and a sub menu. In WebExtension API, you cannot specify the place of a menu item. If you just add one menu item, then it becomes a top-level menu. If you add two or more menu items, then the top-level menu item whose label is the extension name are automatically created and menu items you created are placed as sub-menus.

As for changing the menu item label for expanding the default format to the actual format name, I agree it's nice but it's somewhat difficult to implement. I'll try to investigate in a day or two.

IzzySoft commented 7 years ago

@jflorian testing an addon outside of AMO is prettxy easy. Just clone the repo (or download and unzip the ZIP file), then open about:debugging in Firefox. There you can load a "temp addon", simply navigate to the folder containing its files. If the very same addon is already installed, the temp addon will "replace" it. When done, simply "unload" it at the same place (about:debugging) or restart FF.

@hnakamur I was afraid that WebExt is too limited for even simple things as deciding on top-level and sub-menus. No idea what the Moz guys have between their ears in this regards to decide to, in my eyes, "cripple" the Firefox addon system that much. Thanks for evaluating our ideas! Maybe you let us know (here in this issue) when you've checked in changes for that, so we can take a look at it in the way described above for some "fine-tuning" before you publish?

jflorian commented 7 years ago

@IzzySoft Thanks for the testing how-to. I'll be happy to help evaluate the usability of any changes here. Also, I noticed in the links at the end of your migration list gist, the one to reddit has a spreadsheet there that's grown quite large. Not sure if you were aware of this since they seem like two parallel efforts. It's really sad that Moz did not lead this effort and in a prominent way.

@hnakamur Thanks for the detailed explanation of the WE limitations. Hopefully Moz quickly addresses the most severe limitations. You'd think they'd do that before the sky falls, but probably they tried and nobody responded until actually threatened. I think you've done well against those limitations and I appreciate your hard work to allow us all to be lazy more easily -- in my head that's the whole point of great software! :-)

jflorian commented 7 years ago

Oh, and that spreadsheet is here. (And yes, I did use FL to put it here. :-)

hnakamur commented 7 years ago

I've updated the pull request. Just leave the context menu for the default format and delete other items by hnakamur · Pull Request #11 · hnakamur/FormatLink-Firefox

After playing one context menu item, I feel tedious to push the toolbar button, then a radio button, and then the "Set as default" button. So I deleted the "Set as default" button. Now you can change the default format by two steps, the toolbar button and then a radio button.

Also, when you change the default format, the context menu item label changes accordingly, like "Format Link as Markdown" if you select the Markdown.

But I couldn't make it work for the context menu item label to be changed when I changed the default format in the option page, so I deleted the dropdown list for the default format too. Actually this is redundant since we can change the default format with a radio button, so I think this change simplify the UI even better.

After playing this version, I think the UI is simple enough and I'm satisfied now. @IzzySoft Could you take a look? Thanks!

IzzySoft commented 7 years ago

@jflorian Thanks for checking! And no, I didn't check that spreadsheet (I usually avoid GDocs). As for "parallel efforts": My Gist I originally started just for the addons I used (or found interesting/considered). Just grew a little beyond that when I stumbled upon some other candidates I wanted to keep track of :wink:

@hnakamur that sounds great! Though it means one has to have that additional toolbar button if one wants to change the default format. Not blaming you, rather the limitations of the API. As for "taking a look": Will have to see how I can extract the files from the PR first … Will update here when I'm through.

Update: Done, looks good! Nice idea to have that edit box with the button, so it can serve a double purpose: one could keep a note there or re-edit the description. Not really required, but there might be places where pasting auto-submits, and then one had to be sure beforehand :wink: One thing though: the text box always has the page link – regardless of whether one meanwhile had copied several other URLs. Shouldn't the text in there reflect the latest copy?

Ooops: After updating this post,

screenshot

And it stays that way. Now I cannot switch back to my favorite format (even not on other tabs, new tabs or after reloading the page), as FL is stuck on reST :scream: Not even reloading, unloading and loading it new, or removing it at all did solve it (FL is still installed in the AMO version, so unloading the temp addon only restored the "set as default" button). Looks like all the settings have been lost: Uninstalling the "temp version" and calling the AMO version's settings shows them all empty, and not even "Restore defaults" brings them back. So somewhere there's a bug …

PPS: I cannot even manually add back the definitions. As soon as I press "save changes", all is empty again:

screenshot

IzzySoft commented 7 years ago

Yuck. Fixed that by editing it directly in about:addons :sweat_smile: Not sure if that was triggered by "overloading" the AMO install with the temporary one.

sorry I had to create a new comment for that, but somehow editing my previous one fails

jflorian commented 7 years ago

+1 here also for the edit box. That's a great idea and allows for a quick preview (try before you buy) for working with sites that have their own markup that's similar but not identical. This way I can find one that's close enough without having to think about what each would look like. (The great thing about standards are there's so many to choose from!)

Can this be combined into the actual setup of new formats somehow? I mean if I were to edit an unnamed one, the only missing step would be to give it a name. (Sorry if that's the idea already -- I tried using the just_one_context_menu branch several hours ago but not since.)

IzzySoft commented 7 years ago

Great idea with the "save this new format". Guess the problem with that is to determine the "variables" – after all the box contains the formatted link, not the template, so there must be a safe way to determine the "place holders" (title, url). Considered the entire box is editable, the user could as well have altered title and/or url, so it's more than just a "simple matching".

jflorian commented 7 years ago

I need more coffee. Yup, that's a serious flaw in my idea. I suppose it could have 2 fill-ins and an edit-box, like:

Name: test_____    Format: {{title}}___________
Preview: This is a test link___________________
IzzySoft commented 7 years ago

That makes more sense. But also think of "sanitation". Example: How to format the title [Dev] Link formatting for Markdown? Hint: Square brackets must be masked. So the question is if you want to introduce such complexity into an addon for end users – or rather leave that for a different tool.

IzzySoft commented 7 years ago

@hnakamur

As for menu items, it's not possible to have both a top-level menu item and a sub menu.
In WebExtension API, you cannot specify the place of a menu item.
If you just add one menu item, then it becomes a top-level menu.
If you add two or more menu items, then the top-level menu item whose label is the extension name are automatically created and menu items you created are placed as sub-menus.

That seems to be only partly true – see the Undo Close Tab web extension:

a technical limitation of the WebExtension API which only allows to have 6 items on the first context menu level.

So it would be possible to have up to 6 items in the top-level menu. For my all-day use cases, I'd like to have

which (for me) covers 99%, with the remaining 1% used once a month (so I could toggle it via the "toolbar button" for that). Via the config page, a user could make his/her pick for up to 6 formats to be included with the top level menu, with all other formats going to be available additionally via the "toolbar button". That way everybody could decide to have 1..6 items for "quick access", and all should be happy.

What'd you say?

hnakamur commented 7 years ago

That seems to be only partly true – see the Undo Close Tab web extension:

@IzzySoft Oh, I didn't know that. Thanks for your information. However, after I tried the Undo Close Tab web extension and looked at the WebExtension API documents, the situation for the Format Link extension is still the same.

At parentId in contextMenus.create() parameters,

Note: If you have created more than one context menu item, then the items will be placed in a submenu. The submenu's parent will be labeled with the name of the extension.

You can create up to 6 toplevel menu items if the context is page_action or browser_action.

contextMenus.ContextType - Mozilla | MDN

browser_action Applies when the user context-clicks your browser action. You can only add 6 items to the top-level context menu, but can add submenus.

page_action Applies when the user context-clicks your page action. You can only add 6 items to the top-level context menu, but can add submenus.

The Undo Close Tab web extension creates up to 6 toplevel menu items for the brower_action context. Source code: https://github.com/M-Reimer/undoclosetab/blob/cb3c1a96d60c4fb5f185da7b90e6e33f2c263d9e/background.js#L102-L143

When I pressed the right mouse button on the toolbar button, I saw the multiple toplevel menu items like: https://addons.cdn.mozilla.net/user-media/previews/thumbs/184/184237.png

I did an experiment two menu items with context "link", "selection", and "page" and I confirmed the top level menu item for this extension is created and these two menu items become submenu items. Source code: https://github.com/hnakamur/FormatLink-Firefox/blob/df4263f9c1a2c6bf7e24850dbcbfa7e3c636df08/background.js#L66-L99

You can try it yourself if you install this extension at two_context_menus_experiment branch.

I overhauled the Format Link extension and pushed to hnakamur/FormatLink-Firefox at just_one_context_menu branch. I'll explain about it in a separate comment since this comment becomes long.

hnakamur commented 7 years ago

@IzzySoft said in https://github.com/hnakamur/FormatLink-Firefox/issues/9#issuecomment-327511231

One thing though: the text box always has the page link – regardless of whether one meanwhile had copied several other URLs. Shouldn't the text in there reflect the latest copy?

I agree with you on this for the current modified version of Format Link extension.

This behavior is intentional and valid for the old version.

Let me explain for the history of this extension. At the very first, I created Format Link extension for Safari, which I don't use anymore. git repository for Format Link for Safari. The APIs for Safari extension is even more restricted and they don't allow to copy to clipboard with an API. So all I could do was open the popup when you click the toolbar button, and set up a formatted text in the textarea, and select the text. But the text is NOT copied to the clipboard, so you needed to manually press Control-C to copy the selected text to the clipboard.

Then I created Format Link for Chrome git repository for hnakamur/FormatLink-Chrome. Until recently, it didn't have context menus. It has only the toolbar button and when you click the toolbar button, it copies the URL of the current page and the page title to the textarea in the popup and then select the formatted text and copies the selected text to the clipboard.

And then I created Format Link for Firefox and added the context menu and Alt+C shortcut key. After I implemented and used it myself, it turned out that you cannot get the link URL with pressing the toolbar button nor pressing Alt+C shortcut key. So I've decided to stop copying to clipboard with pressing the toolbar button and delete Alt+C shortcut key. These causes a major incompatible UI change, but I think it is simpler and more consistent. To reflect this major UI change, I'll bump the version of this extension from 1.x to 2.0.0.

So, the context menu is the only way to copy a formatted URL and text to the clipboard now. And yes, with this version, the text shown when you press the toolbar button should be the last copied text. I implemented this at Let toolbar button to use the last copied url · hnakamur/FormatLink-Firefox@6fede74. It uses browser's local storage storage.local to keep the values for URL, title, text variables and the format and the formatted text.

hnakamur commented 7 years ago

About user experiment for the new format with a textarea. I don't think it is good to do that in the textarea in the toolbar button's popup.

If I ever implement this feature, I think it is appropriate to add the sandbox area below the options page. And you need fields for all supported variables as well as a field for format and a field for the result.

URL: http://example.com_____ Title: this is a page title Text: this is a link text
Format: {{title}} Preview: This is a page title____

I think it may be helpful for users, but I don't think it is absolutely necessary since you can experiment on real pages.

IzzySoft commented 7 years ago

@hnakamur thanks for the extensive explanation! I believe you if you say the argument with "6 top-level entries" will not apply here – after all, you're the dev, and I've almost no experience with WebExt development (apart from tinkering with existing code). So it was a nice idea, but no more, unfortunately – unless we've missed some API stuff (should a possibility reveal itself, we can keep that in mind though).

As for the textbox context: Glad I got that right and you consider this must be done. Looking forward to the new version :wink:

I also fully agree with you concerning the "new format", that this should be going to the options page (if it is to be implemented, that is). It's nothing one needs for "daily use", but rather for the rare case one wants to configure a new format. Most users wont do that anyway (or only copy existing formats offered by "power users"). The pre-formatted formats will fit 99%+ – so it shouldn't clutter a "central place" you use all the time (would only disturb us there). So I consider this feature a "nice to have", but not a "crucial must-have".

If you want me/us to test the latest code before you publish: for me that will have to wait a little. Maybe I can take a look during the weekend, but I'm not sure if time permits before Monday.

hnakamur commented 7 years ago

About sanitize. I prefer escape to sanitize.

For example, think about the closing square bracket character ]. Yes it conflicts the markdown URL syntax since ] is used to express the end of the link title. If you go for sanitization, the only you can do is deleting the characters, so you cannot use that characters. If you go for escaping, the closing square bracket character ] is escaped by adding the escape character, that is \ in this case, is added before each ]. In this way, you can use ] in the title.

And this escape feature is the very reason I created my extension. I looked at extensions created by other people, but I couldn't find one with escape feature. I don't want to limit the characters I can use in titles, so I've created an extension myself.

I'm a programmer and familiar with Regular expression - Wikipedia, so I decided to use those to escape conflicting characters.

So I think escape is the way to go. However, if you still want to use sanitize, you can do that with passing the empty string "" for replacement, like [{{text.s("\\[","").s("\\]","")}}]({{url.s("\\)","%29")}})

IzzySoft commented 7 years ago

Escape versus sanitize: Yeah, full ack. Bad choice of words on my end, but that's exactly what I had in mind!

hnakamur commented 7 years ago

@IzzySoft https://github.com/hnakamur/FormatLink-Firefox/issues/9#issuecomment-328227662

If you want me/us to test the latest code before you publish: for me that will have to wait a little. Maybe I can take a look during the weekend, but I'm not sure if time permits before Monday.

Yes, this a major overhaul and it will be version 2.0.0, so I'd like you test it thoroughly. Of course, I tested it myself, and I think it works as expected, but I confess I'm not so doubtful to be a good tester :-/ So your help is very appreciated. No need to rush, test it in your spare time. I don't mind it take time, since I can use the dev version myself.

Also, if you become busy, feel free to say so. Contribution like commenting or testing should be a fun activity, not a job with the responsibility ;-)

IzzySoft commented 7 years ago

@hnakamur Just did a quick check, looks good:

Just checked the description on the addons page, and wondered about the following:

Due to security reason, the limitation becomes severer on about: pages and addons.mozilla.org pages. You cannot use the context menus nor the keyboard shortcut on those pages. You can use the toolbar to copy the URL, but the text always become the page title even if you select some text.

While the first two lines are clear, the last doesn't prove true (or I used the wrong approach): I wasn't able to copy any URL to the clipboard using the toolbar on the about:addons page. Oh, and funny thing: While writing this comment I tried the same on this page here, which also contradicts that 3rd line: the clipboard showed the link "2 days ago" (i.e. the timestamp-to-comment link of your preceding comment). Worked as advertised for other pages, though. So maybe at least add to the last line something like "Doesn't work on about: pages at all"? Didn't check other about: pages (except for about:debugging), but I expect the same behavior on them.

A final thought: should there be an option to have all formats in a context sub-menu (the way the current AMO version has it)? Some folks might prefer it that way. Not saying we must have this, just asking whether we should consider it :wink:

hnakamur commented 7 years ago

@IzzySoft Thanks for your tests and comments!

Also thanks for a catch about the description! I've updated the description at https://github.com/hnakamur/FormatLink-Firefox/pull/11/commits/edf83c031034dbf3c18a0ecd716de26ef514963e In the updated version, the only way to copy a URL is using the context menu, so it does not work at all on all "about:" pages and addons.mozilla.org pages.

The description at https://addons.mozilla.org/ja/firefox/addon/format-link3/ is for the current release version, so the description is not correct for https://github.com/hnakamur/FormatLink-Firefox/pull/11 Once I submit this version and it passes the review, I'll update the description.

I considered about an option, but I don't like it since it makes complicate to maintenance the code and also it takes more time to test the extension every time we update the code.

hnakamur commented 7 years ago

As soon as I released 2.0.0 with only one context menu, I got a request to have context sub menus as before from a user. So I added the "Create submenus" checkbox in the options page and released it as version 2.1.0. Thank you all for your help!

IzzySoft commented 7 years ago

I almost expected that ;) Good idea to have this option. Unfortunate this can't be mixed due to the WE restrictions.

jflorian commented 7 years ago

Just took 2.1.0 for a spin. Fantastic job! To me, this is great example of the "do one thing, do it really well" tenet. I really appreciate the effort.