Port to new WebExtension format

PerfectSlayer commented 8 years ago

Hi all,

Mozilla introduce a new way to develop add-on (again). Here comes the WebExtensions. The main advantages of WebExtension for gTranslate are the following:

Support several browsers
Faster review

But we need to repackage and use the new common API.

There is already some issues that could be fixed with:

65 Firefox Android support
13 Google Chrome compatibility
53 Pale Moon Browser support

And some that could be easier with:

46 Replace the selection by the translation (as WebExtesions know how to deal with Electrolysis)

Moreover, this new extension format seems to be largely adopted by browser in the coming month. It could be a good way to gTranstale to become even more popular throw the other browser.

So what do you think about porting the current add-on ?

bpierre commented 8 years ago

It's really nice to finally see progress towards a standard way of building addons!

I'm all for it, let's see if the API is complete enough to reproduce the current state of the addon.

I don't see any mention of a contextual menu API here, not sure if it's supported yet? https://blog.mozilla.org/addons/2016/04/29/webextensions-in-firefox-48/

PerfectSlayer commented 8 years ago

As far I could see, all the API are here:

context memu
i18n (to get current locale)
storage (to save settings)

Those API work for Firfox (since version 45), Chrome and Opera (since version 33). Do you see anything missing to get the add-on working?

I checked the development tutorials. It seems we just need to repack and use the right API but it should not be so hard. Otherwise, we'll need to find testers for other browsers! 😅

bpierre commented 8 years ago

I checked the contextMenus API, and it looks like there is no support for nested menus at the moment.

PerfectSlayer commented 8 years ago

I think you just have to add a parentId in the createProperties object.

bpierre commented 8 years ago

Ah, I didn’t see that, cool!

PerfectSlayer commented 8 years ago

Yep, everything should be here to start a port :wink:

happysurf commented 8 years ago

Please increase (with an option) the font size of tooltip translation. The rest seems perfect. :-) UPDATE: Fixed in Windows 7 changing the font size in Message Box from Segoe UI 9px to 10px.

fluks commented 8 years ago

Should we postpone adding new features until gtranslate is ported to WebExtension?

Has anyone started working on that yet?

PerfectSlayer commented 8 years ago

@fluks Not me. I didn't had time to look at it. I would like to try and play with the build tools (packaging, lint, cross browser support) before.

bpierre commented 8 years ago

@fluks @PerfectSlayer Same, I haven’t tried yet.

S3ndG commented 7 years ago

Hello! Are you able to create a webextension with the actual build tools? thankee :)

akhal3d96 commented 7 years ago

Starting from Firefox 57, WebExtensions will be the only supported extension type. Desktop Firefox and Firefox for Android will not load other extension types. Shouldn't we hurry a little bit in porting it to the new api?

PerfectSlayer commented 7 years ago

Hi @nemoload, This add-on is an Open Source software. Feel free to contribute and participate to the project as I did the past two years. Personally, I haven't migrate my own add-on that I develop since 2005 and I'm quite bored of the constant changes of framework and the need to rebuild from scratch. But it's nice to see hurry people. I just hope you will put as much energy to port the add-on than remind us to do it quickly. 🙇

mote0230 commented 7 years ago

i'm looking into it today.

akhal3d96 commented 7 years ago

@PerfectSlayer I have no problem in working myself on this add-on. I use it on a daily basis and my web browsing experience depends on it. I'm only afraid if the original maintainers are already porting it and my work would be redundant.

mote0230 commented 7 years ago

ok i did some work, but it looks like there's some problems: Selected text is max 150 chars until ff 56 https://bugzilla.mozilla.org/show_bug.cgi?id=1338898 , making it half useless until then.

Next is there are no events available to notify us the context menu gtranslate item is being expanded, so best we can do is make a translate request to google every time some text is selected and the context menu is opened. This also requires a context-script that will always be watching for these things. I'd like to know what you guys think about this.

It also blocks lazy loading the to and from language lists, meaning that for performance reasons it will be 2 lists of languages, one for the "from" and one for the "to" language. https://bugzilla.mozilla.org/show_bug.cgi?id=1215376

Less important, no way to not show "translate page" on single images by looking at the mimetype, see https://bugzilla.mozilla.org/show_bug.cgi?id=1361447

There's a branch on my profile, but it's very much work in progress.

PerfectSlayer commented 7 years ago

@nemoload ~~As far as I know, the original maintainer does not have a private development branch (in which he could have started a web-extension port).~~ But he has!

@mote0230 Thanks for your prototype! But it's sad to see the API are still not polished...

make a translate request to google every time some text is selected and the context menu is opened

For me, I will clearly be a no-go! I won't use an extension that will send to an external service provider any text I'm trying to copy through the context menu. But may be other are okay with this behavior… 😫

bpierre commented 7 years ago

Not working on it at the moment, if anyone wants to start, no problem for me!

I started some time ago to experiment with it, and the things I noted are:

As @mote0230 said, there was no way to update a menu after opening it, which is a big issue for this addon.
Minor issue, but there was no way to set a different icon than the addon icon. It appears to be fixed now: https://bugzilla.mozilla.org/show_bug.cgi?id=1321544

mote0230 commented 7 years ago

And another issue that mozilla need to fix first: display the full text in a tooltip. https://bugzilla.mozilla.org/show_bug.cgi?id=1332270

mote0230 commented 7 years ago

And the guy from the context menu tooltip doesn't want to implement it. Only other way I could think of is this:

bpierre commented 7 years ago

@mote0230 Is that possible? I thought there was a length limitation.

bpierre commented 7 years ago

I opened a bug for the “onMenuOpen” event: https://bugzilla.mozilla.org/show_bug.cgi?id=1390061

mote0230 commented 7 years ago

Yeah i put in a string split function for the max length and \n and then a loop to create the entries.

I don't really understand why you need the onMenuOpen. I think all we need is in https://bugzilla.mozilla.org/show_bug.cgi?id=1215376 ?

bpierre commented 7 years ago

I don't really understand why you need the onMenuOpen.

There is no need, I missed this link. Closed 👍

marczellm commented 7 years ago

Hi,

This addon is great and it's the only reason I won't update to Firefox 57 yet. Is the porting work ongoing? I might be able to contribute, though only a few months later.

Scaurus commented 7 years ago

Good afternoon, tell me please, but there is a branch or pull request with a change that could be collected and seen, tested.

def00111 commented 7 years ago

Maybe you can make something like this [1]? [1] https://addons.mozilla.org/firefox/addon/simple-translate/

maicol07 commented 7 years ago

Hi, Firefox 57 is here. I updated to this new version but I can't use your extension because is obsolete... Could you update it? Thanks

STeeL835 commented 7 years ago

What if leave the same functionality as it was before WebExtentions, but if selected text is too long for menu item, add another submenu item that after clicking on it would show some really simple popup under selection right on the page? Or something like that, it just what came in my mind as a substitution for the context menu tooltips. And I don't know if it's reasonable to use for showing other translation options..

bpierre commented 6 years ago

An update from the Mozilla Add-ons Twitter account:

Hey there! Bug 1215376 might be landing as early as FF 59. Unfortunately, bug 1332270 is a WONTFIX. We think there’s another way you can do that, though.

Want to get in touch? We’d love to talk more about workaround ideas - https://wiki.mozilla.org/Add-ons/developer/communication#Get_in_Touch

From https://twitter.com/mozamo/status/933394486859767810

marczellm commented 6 years ago

I'd love to have this as WebExtension even at the loss of the tooltip functionality.

marczellm commented 6 years ago

onBeforeShow is now implemented and coming to Firefox 60.

mote0230 commented 6 years ago

Had a look at it again today, we're still stuck. onShow only triggers when the context menu is opened and there's no way to make it listen to our submenu's, so we're no further there. Then I couldn't find a way to get the selected text in a variable, the only moment it seems where's that's possible is in onClick. You can display it in a menuItem by using %s but then you can't use it, and I didn't see a way to 'get' the menuItem and read its text.

marczellm commented 6 years ago

I created a basic port at https://github.com/marczellm/gtranslate/tree/webextension. If someone is reading this, maybe you could check it out, look for bugs and missing functionality before I do a pull request.

What it cannot do because of missing APIs:

Display translation of right-clicked link text in context menu (like it does with translated selection)
Tooltips

What it does differently:

Language selection lives under addon settings, not in the context menu (I think it’s a better place for that)

marczellm commented 6 years ago

You can test it on latest Firefox Nightly. Features to the menus.onShown API are still being added.

maicol07 commented 6 years ago

@marczellm how to install it?

marczellm commented 6 years ago

Clone the GitHub repo locally
Install Firefox Nightly
In Firefox Nightly, open about:debugging
Click „Load temporary Add-on”
Select any file from the add-on’s directory

maicol07 commented 6 years ago

Does it work on Firefox 58.0.2?

marczellm commented 6 years ago

No

maicol07 commented 6 years ago

Ah..

fluks commented 6 years ago

Works. @mote0230 mentioned a way to show a longer text, maybe that could be integrated? Now if the selected text is long, it only shows "Fetching translation...".

marczellm commented 6 years ago

For a long text, Google Translate seems to return no translation in the JSON response, only nulls and empty strings.

Edit: It works in the old version of the addon so there's something I messed up.

mote0230 commented 6 years ago

Starting from @marczellm 's code, i fixed the multiline as in the screenshot in https://github.com/bpierre/gtranslate/issues/66#issuecomment-321987010 , and enabled alternative translation and dictionary stuff for single words by default, since i couldn't quickly figure out the options and i have to go outside now. Remember that it still sends your selection to google every time you have text selected and right click. https://github.com/mote0230/gtranslate/tree/webextension

marczellm commented 6 years ago

I fixed the long text problem

marczellm commented 6 years ago

Dear @bpierre,

Firefox 60 launches on 09 May. I have ported gtranslate to WebExtension minus the tooltip functionality. @mote0230 implemented a workaround: the description lines can be displayed as a list of noop context menu items.

Would you like us to create a pull request so that you can release the WebExtension version as an update to the existing addon, or should we release it separately, named gtranslateWE or similar? (as happened to some other addons)

marczellm commented 6 years ago

(@mote0230, I intend to pull your changes into my fork and polish its relations to the options screen, but only after 09 May)

achernyakevich-tch commented 6 years ago

Concerning to comment of @mote0230:

Remember that it still sends your selection to google every time you have text selected and right click. https://github.com/mote0230/gtranslate/tree/webextension

I have just added my comment to the pull request https://github.com/gtranslate/gtranslate/pull/86, probably it will be more correct to add it here:

Maybe desired feature could be achieved by slight change of user behavior. For example, the following case could be valuable for me:

I do some selection on screen

I make right click

context menu is displayed

I navigate to gTranslate's menu item Translate ...

I click this menu item

on click extension will call Google Translate service, build next level submenu and show to me required information

So difference is only "click/press Enter" vs "on mouse over/selection item by keyboard".

From other side it will completely avoid problem of sending of selection to Google every time when context menu is called.

marczellm commented 6 years ago

@achernyakevich Correct me if I'm wrong, but I think if you click any menu item, the menu is dismissed

achernyakevich-tch commented 6 years ago

@marczellm

I have just tested in my FF 56.0.2 and in FF 59.0 - in both cases if menu item contained submenu then click caused no collapsing but opening of a submenu.

I can't perform more reliable checking for case when menu item does not contain submenu items and by clicking it will build submenu and will prevent collapsing of menu in general. But I can expect that return false for the even handler could reasonably prevent collapsing of menu. I think you could easily test by small peace of code (sorry, I have no infrastructure for web-extensions development).

In any case, gTranslate already has some submenu. So if we will be able to catch click/press Enter then we could try to rebuild submenu. Having "..." or "(click this item)" text in menu item we could inform user to make a click/press Enter.

marczellm commented 6 years ago

@achernyakevich

return false does not prevent menu dismissal.
Cannot catch a click on a menu item that has a submenu.
You don't need infrastructure, only a text editor.

bpierre / gtranslate

Port to new WebExtension format #66

65 Firefox Android support

13 Google Chrome compatibility

53 Pale Moon Browser support

46 Replace the selection by the translation (as WebExtesions know how to deal with Electrolysis)