mozilla / nightlytt

Nightly Tester Tools
https://wiki.mozilla.org/Auto-tools/Automation_Development/Projects/Addons/NightlyTesterTools
Other
62 stars 37 forks source link

Rewrite NTT as webextension #235

Closed whimboo closed 7 years ago

whimboo commented 7 years ago

NTT is still an old-style extension which uses XUL overlays. This won't be supported anymore with Firefox 57. So what we need is a rewrite of the whole extension as webextension.

Not sure if everything which we currently support can be implemented, but we should try to get as much as possible ported over.

So far I found those deficits when checking the API:

Good things which will work:

So seeing this list we most likely will loose a lot of our features, and I wonder how useful such a rewrite will be.

@pascalchevrel we still list this addon on the nightly page. I wonder how important it is for you all those days. Personally I don't have the time to do the rewrite. Also CC'ing @AutomatedTester and @stuartphilp for info.

Statistics for the add-on can be found here: https://addons.mozilla.org/en-US/firefox/addon/nightly-tester-tools/statistics/?last=30

tonymec commented 7 years ago

Firefox 57 already! Does that mean all Toolkit 57 applications, or will there be a delay for Thunderbird and/or for SeaMonkey? IIUC, both Lightning and Chatzilla are also under the crosshairs.

whimboo commented 7 years ago

I don't know if and when other toolkit applications will be affected. There might be similar blog posts like https://blog.mozilla.org/addons/2017/02/16/the-road-to-firefox-57-compatibility-milestones/ which explain the process.

pascalchevrel commented 7 years ago

@whimboo I checked with marcia and a lot of our testers use this extension so a rewrite as a web extension, even with a reduced set of features, would be useful I think.

xabolcs commented 7 years ago

Uhh-ohh, no Chatzilla for Fx57? That will be sad.

@whimboo, what about starting a new NTT fork? There will be two repo, one for this webextension rewrite (e.g. Nightly Tester Tools WebeExtension), one for the toolkit applications (e.g. Nightly Tester Tools for Toolkit)? There is already an NTT for Mobile. :)

I would like to keep supporting Thunderbird and SeaMonkey.

whimboo commented 7 years ago

We will keep this repository. I don't see a need for another one. We will simply create a new branch while the webextension is under development. Later we can flip and keep a branch for the old-style add-on.

The biggest question for now is how we can manage the rewrite. As I mentioned earlier I most likely won't have the time to work on that. So we need some people to actually do the work.

xabolcs commented 7 years ago

I don't see why should live two separate product in the same repo.

Anyway, I happily do the experiment. In the mean time it would be nice to define what should we call 'done' for this issue in the description.

whimboo commented 7 years ago

It will still be NTT. It's not a different product / fork. It's just built on a different code base. I strongly wish to be able to keep the current extension id if that is possible.

We will most likely have a couple of different PRs. Small ones are definitely appreciated so that development can proceed faster. If you would really have the time to work on that it would be fantastic! Maybe some other contributors might also join in once the basics are done, and development for different features can happen in parallel.

xabolcs commented 7 years ago

It is. NTT-webex supports only Fx 57 and up with a feature set. NTT-toolkit supports anything but Fx57 and up with a more wider feature set (including UI) compared to the webex version.

OK, will provide a starter PR sometime. Please note that is just mean I start experimenting ... feel free to jump in!

UtiluMark commented 7 years ago

The ability to change and customize the title of a window to include the exact version information is quite an essential feature of NTT for me. A WebExtension version of NTT without this feature wouldn't be very useful.

whimboo commented 7 years ago

This rewrite is not optional, or based on personal opinions. It's something which has to be done, or the add-on will no longer exist. @UtiluMark, if you want to help to make the best out of it, I would appreciate that.

xabolcs commented 7 years ago

It's not so simple, IMHO. If rewrite happens then the add-on will no longer exist for Toolkit Applications.

UtiluMark commented 7 years ago

If the WebExtension version of NTT will get the ability to change and customize the title of a window I would like to help, but what I meant was that without this essential feature NTT just wouldn't be useful for me.

whimboo commented 7 years ago

Maybe we should consider to create an Embedded WebExtension so that we can port the user data over to the new extension. Details are layed out here: https://blog.mozilla.org/addons/2017/01/20/migrating-to-webextensions-port-your-stored-data/.

I found https://bugzilla.mozilla.org/show_bug.cgi?id=1268020, which would allow us to use an entry under the Tools menu once implemented.

Btw. there are also office hours when help can be requested: https://atsay.github.io/WebExtensions_office_hours/

xabolcs commented 7 years ago

Users will lose lot more feature, not comparable to the user data. :trollface:

How about waiting for another year for more complete WebExtensions API? The comparison table is full of None. There are a some bugs which could be fixed for the non-Fx57+ users.

Anyway, the office hours are great, surely will join to IRC when the time comes! 👍

whimboo commented 7 years ago

I experimented a bit today and will push something up to its own branch maybe tomorrow. As testing has shown the best option for now is to use the context menu API, even that our menu will be displayed only when doing a right click in the content. There is no support yet for real menus, and using a toolbar button only allows us to use a popup which needs much more work.

whimboo commented 7 years ago

@xabolcs I forgot to upload my experimental code which I was working on half a month ago. I created https://github.com/mozilla/nightlytt/tree/webextension on this repository now. It should be great to get started. The copy to clipboard feature is only half-wise implemented and misses the actual clipboard code.

Otherwise it includes:

yfdyh000 commented 7 years ago

FYI: Bug 1333376 Feature request: a WebExtension API to change the window title REOPENED Bug 1268020 Implement "tools_menu" context ASSIGNED

whimboo commented 7 years ago

Thanks. I updated the original comment with that info.

whimboo commented 7 years ago

Just to update https://bugzilla.mozilla.org/show_bug.cgi?id=1333376 landed on mozilla-central today. As such we will be able to set a window title prefix with the webextension! \o/

Anyone who could help to port features over to the webextension?

tonymec commented 7 years ago

At the moment (at least in SeaMonkey 2.53a1) having a WebExtension enabled causes a shutdown crash, see https://bugzilla.mozilla.org/show_bug.cgi?id=1379994

For this reason I had to downgrade Adblock Plus to version 2.8.2 and now every time that I check for add-on upgrades, the add-on manager displays an update to Adblock Plus which I have to deny. I don't want to have to "freeze" NTT updates for the same reason. You may want to follow that bug to see if and when it gets fixed.

kyoshino commented 7 years ago

@pascalchevrel asked me on IRC to help the WebExtension port, so I just started with a fresh repository at https://github.com/kyoshino/nightlyttr

whimboo commented 7 years ago

@kyoshino this is fantastic! Please note that we already have https://github.com/mozilla/nightlytt/tree/webextension with working code. It's not that far yet, but you can continue there.

whimboo commented 7 years ago

@kyoshino would you mind sharing with us which state the webextension has? Is there anything we should try out? I have a problem building the XPI locally with the web-ext tool because it always gives me a broken archive. Thanks.

Also please note that over the weekend we stopped supporting legacy extensions and only allow webextensions. You will see this in about:addons with nearly everything been removed behind a second page.

kyoshino commented 7 years ago

I'll finish my port this week by investigating what can we do more with the limited API sets. The most things are already noted in your initial comment though. Then I'll send a pull request to this repo to merge my fork, so it will be ready for publishing on AMO. I haven't used the web-ext tool yet :blush:

whimboo commented 7 years ago

That's great. Please let us know when we can have a first look at it and give feedback. By the first look I have seen that you haven't kept the entry in the tools menu. I would suggest we keep that given that users are using it since the beginning. In the toolbar we only had a button to easily access the copy/insert build id feature.

Regarding the web-ext tool, you should definitely use it. It makes it so much easier with the auto-reload.

kyoshino commented 7 years ago

Yeah I thought a toolbar button might be more handy but will take a look at how to add items to the Tools menu as well. Bug 1268020 has been solved but not yet documented. Also, I'll probably add context menus to insert the build ID to a textbox.

whimboo commented 7 years ago

As mentioned 13 days ago please have a look at https://github.com/mozilla/nightlytt/tree/webextension. This implements the tools menu already.

kyoshino commented 7 years ago

Cool, thanks for the pointer.

kyoshino commented 7 years ago
kyoshino commented 7 years ago

I'll try to add other features through WebExtensions Experiments

So here's my initial attempt: https://github.com/kyoshino/nightlyttr/compare/experiment

This should work, but the installation fails likely due to a bug in Firefox (this Hello World sample also fails). Will figure out the regression range.

kyoshino commented 7 years ago

I just uploaded the WebExtension to the beta channel on AMO. Give it a try!

Cai0407 commented 7 years ago

I just uploaded the WebExtension to the beta channel on AMO. Give it a try!

Excellent! I have confirmed that NTT 4.0pre20170912 seems to work fine with Nightly 201709121000139 on Win7.

whimboo commented 7 years ago

I merged PR #245 to master now. I will file new issues for things missing for the 4.0 release. Please check the 4.0 milestone issue list.

whimboo commented 7 years ago

@kyoshino I tried to install the beta for Firefox 56 but it's not allowed. Do you know if there is a restriction somewhere? Maybe it is because it's not signed? That way it will be hard for our users to actually test this version.

xabolcs commented 7 years ago

I installed when it's pushed to AMO and now too ... without problems. FDE/56.0b11

kyoshino commented 7 years ago

It works for me on both 56 Beta and 56 Developer Edition.

whimboo commented 7 years ago

Oh, it was the Random User Agent spoofer extension which had Firefox 51 selected. Disabling it made it work.