ESLint documentation moved from MDN and updated

[Mark Banner]

I've just transferred the ESLint documentation that was on MDN and 
integrated it in the existing documentation on Firefox source docs.


This has also been updated, and the ESLint page 
 
now has an updated section on common issues and how to solve them, how 
to deal with es6 modules and how to resolve various issues with no-undef.


You can find the general information about linting here (including where 
to get help):


https://firefox-source-docs.mozilla.org/code-quality/lint/index.html

and the more specific information about ESLint here:

https://firefox-source-docs.mozilla.org/code-quality/lint/linters/eslint.html

Mark

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Please run mach bootstrap; NodeJS/NPM security fixes landed

[Mark Banner]

  * Upgrades for NodeJS from 10.21.0 to 10.22.1 and for NPM from 6.14.4
to 6.14.6 have merged to mozilla-central.
  * Everyone is encouraged to run `mach bootstrap` to upgrade the
toolchain on their machine.
  * The main security fix that we’re concerned with is in node, so I’ve
also set 10.22.1 as the minimum acceptable node version that mach
commands will allow.

This means that running mach `mach eslint —setup` will abort until you
upgrade your node (i.e. you run `mach bootstrap`, in most cases).

https://bugzilla.mozilla.org/show_bug.cgi?id=1666172 has more details.

Mark

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to unship: window.external.AddSearchProvider

[Mark Banner]

On 28/04/2020 16:22, Tom Schuster wrote:
> As the author of one of these extensions for adding a custom search
> engine 
> (https://addons.mozilla.org/en-US/firefox/addon/add-custom-search-engine/)
> I am of course disappointed, but not actually surprised. This is
> basically round two from about a year ago. I would like to point out
> that it would be very simple to only expose this API to extensions. I
> am going to fallback to  for now, which is sadly a
> lot less obvious to users.
One of the problems with addSearchProvider is that it is basically a
loophole for extensions to do things we wanted to be able to stop as
part of the move to WebExtensions - namely being able to install search
engines that survived removal or block listing of the add-on. We've seen
many malicious add-ons, and cutting down the potential routes is a
benefit to users.
> One reason the previous try of deprecating this API was reverted was
> the usage of AddSearchProvider by mycroftproject.com/. I see you
> didn't address this issue in your intent to unship.
We did not feel it necessary to call out the Mycroft Project
specifically, as Telemetry suggests usage is limited (especially if you
exclude clones of Google). We did specifically give options for
websites, which would also be available to the Mycroft project, even if
they are more complex to make work.
> I agree that we should probably remove this API for normal web-pages
> considering the potential for abuse and just the general annoyance of
> prompts. But again my plea: please consider adding proper custom
> search engine support to Firefox itself. Even Fenix has support for
> adding custom search engines! Not even talking about probably every
> other Chrome based browser on Desktop. This is seriously a missing
> piece of customization in Firefox.

Customisation of search keywords/aliases/engines is something we are
planning on looking at later in the year. We have no specific details at
this stage.

Mark.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Intent to unship: window.external.AddSearchProvider

[Mark Banner]

As of Firefox 78, I intend to change `window.external.AddSearchProvider`
in Firefox to be a dummy function. This will be a preference switch
initially, with the original implementation code being removed fully in
Firefox 79.

/Status/:

  *

The HTML Standard specifies this method
as
"must do nothing".

  *

Internet Explorer: This feature was supported in IE7-9 but
deprecated in IE10+ and not present in Edge.

  *

Chrome: Changed to no-op in 54.

  *

Safari: No support.

Product: Mike Connor.

Bug to unship: Preference disable
, Remove code and
preference .

Reasons: `AddSearchProvider` allows adding OpenSearch providers from a
website page. This has been deprecated by the WHATWG, and IE and Chrome
no longer support it. As far as I know it has never been supported on
Mobile.

This API allows a website to put up unsolicited repeated prompts to
users. It is vulnerable to potential DoS
attacks
.

For websites wanting to provide their own engines, the alternative is to
include the  tag, or to provide their own add-ons
which add search engine providers.

Add-ons that use the API would no longer work. Of the two add-ons we
have found that use the API, they are both ways of adding custom search
engines. They both have small numbers of users. Whilst we acknowledge
this will remove some functionality for users, we would encourage users
to request that websites provide their own search integrations which
would have the advantage of being maintained by the website, and being
available to everyone.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

ESLint: nullish coalescing and optional chaining operators now available for use

[Mark Banner]

Last week we finished landing support for the nullish coalescing

and optional chaining

operators in ESLint and Prettier. The operators are recent additions to
Javascript.

We're still lacking support for globalThis (tracking bug
), it looks like
this might be in the next major release of ESLint which is likely to
happen in the next couple of months.

If you have questions about ESLint's support, please pop into
#lint:mozilla.org on Matrix. If you find a bug, please file it here
.

Thank you to Dave Townsend and Ed Lee on working on incorporating the
support for these new operators.

Mark.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

ESLint now enabled for all of mozilla-central

[Mark Banner]

Yesterday, I landed the last patch to enable ESLint on all our
javascript code in mozilla-central*. You can read more about this in my
blog post

(tweet ).

A big thank you to all those that have supported and helped with this
effort.


A note: Please do not add more items to .eslintignore

unless they are:

  * Preprocessed files, where we can't avoid the preprocessing.
  * Intentionally invalid syntax files, e.g. for tests.
  * Generate files that have to be checked in for some reason.
  * Imported code that is linted and managed in other repositories that
belong to us (and for some reason ThirdPartyPaths.txt doesn't apply).

Imported code that is purely third party should be listed
inThirdPartyPaths.txt


Mark

* Except reftests and crashtests. These are intentionally not linted as
they are specially crafted tests.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to ship: Optional Chaining Operator

[Mark Banner]

ESLint doesn't implement new features until they are stage 4, so it
tends to lag behind a bit. Hence at the moment, you won't be able to use
these in our code.

ESLint's tracking issue is here
, and it looks like
they've started working on it, so hopefully it'll be implemented soon.

Mark.

On 22/01/2020 15:34, Johann Hofmann wrote:
> Just a note: Please be conservative about using this in m-c while it's
> not enabled in release yet, to avoid issues when uplifting patches.
> Otherwise, I think this is a great new feature that I'd love to use.
>
> On Wed, Jan 22, 2020 at 4:30 PM Patrick Brosset  > wrote:
>
> Thanks Yulia, this is going to be very useful and helpful in
> reducing code
> complexity!
>
> For info, here's a DevTools bug to make sure the console panel still
> supports autocompletion even when ?. is used:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1594009
> I also a question: do you know if eslint is ready for this new
> syntax? In
> other words, can we use it in m-c without causing eslint parsing
> errors?
>
> On Tue, Jan 21, 2020 at 8:11 PM Yulia Startsev  > wrote:
>
> > Hi,
> >
> > In Firefox 74, we'll ship the optional chaining operator.
> >
> > *Bug: *https://bugzilla.mozilla.org/show_bug.cgi?id=1566143
> > *Standard: *https://tc39.es/proposal-optional-chaining/
> > *Platform coverage: *All, no pref
> > *DevTools bug: *N/A.
> > *Other browsers:* Shipping in Chrome, Shipping in Safari
> > *Testing: *
> >
> >
> 
> https://github.com/tc39/test262/tree/master/test/language/expressions/optional-chaining
> > <
> >
> 
> https://github.com/tc39/test262/tree/master/test/language/expressions/coalesce
> > >
> >
> > *Use cases: *Optional chaining may be useful in cases where a
> property may
> > be conditionally present. This affects property access, dynamic
> property
> > access, and function calls in the following way:
> >
> > *Property access*
> >
> > Before
> > const street = user.address && user.address.street;
> >
> > After
> > const street = user.address?.street;
> >
> > *Dynamic Property Access*
> > Before
> > const street = user.address && user.address["street"];
> >
> > After
> > const street = user.address?.["street"];
> >
> > *Optional function calls:*
> >
> > Before
> > const result = myObj.method && myObj.method();
> >
> > After
> > const result = myObj?.method();
> >
> >
> > *Secure contexts:* This is a JS language feature and is
> therefore present
> > in all contexts.
> > ___
> > dev-platform mailing list
> > dev-platform@lists.mozilla.org
> 
> > https://lists.mozilla.org/listinfo/dev-platform
> >
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org 
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

ESLint: Third-party paths are now obtained from ThirdPartyPaths.txt

[Mark Banner]

tl;dr ESLint has just moved reading third-party paths from .eslintignore
to ThirdPartyPaths.txt.

A couple  of bugs
 have just landed
on central which have updated ESLint and changed how we handled
third-party paths.

The new ESLint changes mean that we are able to load paths to ignore
from external files and hence we now load them from
tools/rewriting/ThirdPartyPaths.txt
.
Obviously this now means we only need to list them in one place.

Please only add files/paths to .eslintignore for things we specifically
don't want to ESLint only - for example, preprocessed files or files
that intentionally have invalid syntax. Anything that is third-party
should now go into ThirdPartyPaths.txt.

Thanks,

Mark

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Recent ESLint updates - BigInt, improved autofixing and more

[Mark Banner]

Over the last couple of weeks we landed a couple of ESLint updates which 
support new features and fix a few issues.


Here's a quick summary:

 * BigInt
   

   is now supported.
 * For globals defined via /* global foo */ statements but which are
   unused, we now throw an error.
 * If you use --fix across the whole tree, it should now work in
   one-shot for all supported files.
 o Previously you'd need to run it on multiple directories (e.g.
   services, toolkit - anything after 'js' in the alphabet).
 * no-async-promise-executor
    is a new
   rule which aims to avoid issues with having asynchronous functions
   within new Promise().

Mark

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

PSA: Changes to ESLint & HTML mainly affecting Sublime Text users

[Mark Banner]

tl;dr We've changed ESLint's configuration to enable HTML parsing by 
default for better editor integration. Sublime Text users will need to 
change configuration.


Up until now the ESLint configuration in mozilla-central has been with 
the html plugin not enabled, but to enable it when `./mach eslint` is 
run. The main effect of this is that editors that automatically run 
linters, HTML files would not be checked.


With the configuration change 
, HTML files will 
now be parsed by VSCode and Atom (and possibly other editors).


The downside is that Sublime Text has its own parsing for HTML files and 
this conflicts with ESLint's HTML plugin 
.


If you want Sublime to keep working, you need to change your user 
settings for the SublimeLinter plugin to include the selector 
configuration below:


|{ "linters": { "eslint": { "selector": "source.js, text.html.basic, 
text.xml", } } } |||


Mark

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

New xpcshell-test option to aid debugging failures when run in parallel

[Mark Banner]

tl;dr We now have a --verbose-if-fails option that outputs a log if a 
xpcshell-test fails when being run in parallel.


If you ever have seen output from xpcshell-tests like this:

 0:05.68 TEST_START: 
toolkit/components/search/tests/xpcshell/test_remove_profile_engine.js
 0:08.57 TEST_END: FAIL
 0:08.61 INFO 
toolkit/components/search/tests/xpcshell/test_remove_profile_engine.js failed 
or timed out, will retry.
 ...
 0:19.50 INFO Retrying tests that failed when run in parallel.
 0:19.50 TEST_START: 
toolkit/components/search/tests/xpcshell/test_remove_profile_engine.js
 0:21.27 TEST_END: PASS

and wondered if that's an intermittent you're trying to fix or just why 
is that failing... now you have a new tool.


--verbose is typically too noisy as it outputs all the logs of the 
parallel tests at the same time.


I've just added the new option --verbose-if-fails which will output logs 
for any failing test(s) during the parallel run.


 0:07.72 TEST_END: FAIL
 0:07.75 INFO >>>
 0:06.49 INFO (xpcshell/head.js) | test MAIN run_test pending (1)
 0:06.49 INFO (xpcshell/head.js) | test run_next_test 0 pending (2)
 ...More logs...
 0:07.79 INFO <<<
 0:07.79 INFO 
toolkit/components/search/tests/xpcshell/test_require_engines_in_cache.js 
failed or timed out, will retry.
 ...

I've used this style of output in the past to figure out why some of our 
search tests were taking a long time to timeout, then would pass on the 
second run. This is just formalising it into a usable option.


Mark.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

PSA: .eslintrc.js files are no longer needed in test directories (in most cases)

[Mark Banner]

tl;dr I've just landed bug 1415265 
 which means you 
no longer need .eslintrc.js files extending the configuration in 
commonly-named directories.


Since early in the ESLint days, we've needed extra configuration files 
in our test directories to ensure that we have the right environment and 
rules set up for them.


This is no more! Following a hint 
 on 
the ESLint issue that was preventing this, we've been able to work 
around the issue, and removed about 200 of the .eslintrc.js files.


If your test directory is named matching one of regexps below, then the 
configuration will be automatically applied:


 * xpcshell:
 o **/test*/unit*/
 o **/test*/xpcshell/
 * Browser-chrome mochitest:
 o **/test*/**/browser/
 * Chrome mochitest:
 o **/test*/chrome/
 * Plain mochitest:
 o **/test*/mochitest/

These can be found in the top-level .eslintrc.js file.

I've commented on revisions already in Phabricator where I know they are 
now adding files unnecessarily. I'll keep an eye out for any more.


For the next steps, I'm planning on making more of our directories 
conform to this format where it makes sense. A lot of the remaining 
cases either have no named subdirectory (e.g. just test/) or are sharing 
multiple test types in one directory (this latter case makes it very 
hard to get the ESLint configuration right).


Mark

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: PSA: Dangling Commas landing part 2 - 20th Oct

[Mark Banner]

On 03/10/2018 15:45, Mark Banner wrote:


Following the successful enabling of the ESLint comma-dangle rule for 
some of our javascript files, I am intending on enabling it for the 
rest on Friday 20th October.



...
Bug 1486741 <https://bugzilla.mozilla.org/show_bug.cgi?id=1486741> is 
the tracking bug for this, and there's a try push 
<https://treeherder.mozilla.org/#/jobs?repo=try&revision=326e8ccf859084fe4632420e308d244b75c546bb> 
from earlier today with the first version of the patches. 


This has now landed on autoland.

Mark.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

What's next for ESLint on Firefox Source Code?

[Mark Banner]

A couple of days ago I blogged about what's next for ESLint on 
mozilla-central. In case you haven't seen it already, you can read about 
it here:


https://www.thebanners.uk/standard8/2018/10/03/whats-next-for-eslint-on-firefox-source-code/

Feedback welcome, on the blog or direct to me.

Mark.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

PSA: Dangling Commas landing part 2 - 20th Oct

[Mark Banner]

Following the successful enabling of the ESLint comma-dangle rule for 
some of our javascript files, I am intending on enabling it for the rest 
on Friday 20th October.


This is at the end of the soft freeze period, but just before the merges 
happen, to make uplifts easier.


This will affect any directory that has ESLint enabled (aka not 
mentioned in .eslintignore) and isn't browser, services, toolkit or 
taskcluster.


Bug 1486741  is 
the tracking bug for this, and there's a try push 
 
from earlier today with the first version of the patches.


Mark.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Looking for an implementer for a improvement to SublimeLinter-ESlint

[Mark Banner]

Hi all,

I am trying to improve our ESLint configuration, especially around HTML 
files. One of the things that limits us in the ideal scenario is the 
SublimeLinter-ESLint plugin.


The plugin currently does its own work for html files which stops 
working when eslint-plugin-html is enabled (which we want to enable by 
default in the mozilla-central configuration). I've been discussing it 
with the SublimeLinter-eslint team, and they have accepted an idea for 
an implementation for an automatic detection method so that it detects 
the ESLint plugin and stops doing its own html processing.


At the moment, this is not something I really want to try and tackle, so 
I'm looking for someone who might like to take it on.


We have a description of what needs changing, which is in the plugin 
itself, and is python based. Here's the details of the bug and the fix 
needed:


https://bugzilla.mozilla.org/show_bug.cgi?id=1464007

https://github.com/SublimeLinter/SublimeLinter-eslint/issues/253#issuecomment-416910981

If there's anyone interested in taking this on, please let me know.

Thanks

Mark.



___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Plan for Sunsetting MozReview

[Mark Banner]

On 05/09/2018 04:40, Kris Maglione wrote:
Concur. Aside from future-proofing things, reading comments in 
phabricator is pretty painful, especially for bugs with multiple 
patches. With the old flow, I could look at all of them in one place. 
Now, I have to open a half dozen separate pages, and then scroll 
through the entire patch squinting for comments, since none of the 
comments at the top of the review page provide even the slightest bit 
of context.


A couple of things that may help with the scrolling & finding, that 
people may or may not have found yet...


- You can click on the line number to the left of a comment to jump 
directly to the comment.


- Once you're in the patch section of the page, there's a bar that 
appears at the top. Click the "x / y comments" button and you'll be 
taken straight to the next open comment and hence you can cycle through 
all the open comments quickly (though it doesn't seem possible to go 
through the "done" ones in this way).


Mark.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Proposal: Require dangling commas for multiline objects/arrays on all Javascript mozilla-central code

[Mark Banner]

On 22/08/2018 21:29, Mark Banner wrote:


Since there are a large amount of instances (approx 12k) that would 
need to be fixed across the code base, I would propose that we roll it 
out on a per-directory basis. For example, we could do all of browser/ 
at the start of the 64 cycle just after the merges (alternately the 
end of the 63 cycle during soft freeze), then do toolkit/ at the next 
merge time, and then pick up the rest at the end. This will limit the 
scope of possible bitrotting to smaller chunks, as well as making the 
patch sizes more manageable.


I've not heard any negative comments, so I am moving forward with this. 
Having looked at the stats a bit more, I'm going to land this in two 
chunks. The first will be to enable the rule on files in these 
sub-directories of mozilla-central:


browser/ services/ toolkit/ taskcluster/

I'm aiming to land these on Friday (31st August), ahead of the 63 merges 
to beta. As others suggested, this should make uplifts easier. Bug 
1486739 <https://bugzilla.mozilla.org/show_bug.cgi?id=1486739> is 
tracking this.


For the rest of the files, I'm currently looking at Friday 19 October, 
ahead of the 64 merges to beta. Bug 1486741 
<https://bugzilla.mozilla.org/show_bug.cgi?id=1486741> will track this part.


Mark.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Proposal: Require dangling commas for multiline objects/arrays on all Javascript mozilla-central code

[Mark Banner]

I would like to propose that we require dangling commas for multi-line 
object/arrays in Javascript code for mozilla-central.


Why:

 * Multiple components in toolkit/ and browser/ already have
   comma-dangle enabled
   
.
 * Having asked around in a few locations, I'm seeing support from the
   Firefox developer team and others in favour of enabling it.
 * It helps make blame cleaner.
 * It makes editing easier, and helps for a consistent style.
 * Having automation available to cover the requirement reduces the
   need for review nits.

I realise not everyone necessarily loves the style, but having the 
cleaner blame, and consistent style requirements seems to outweigh the 
downsides.


How:

 * We would enable this via an ESLint rule, comma-dangle
   , with the
   "always-multiline" option set.
 * ESLint is able to automatically fix code that doesn't conform (e.g.
   bug 1476228 
   was entirely automatically generated very quickly)

Since there are a large amount of instances (approx 12k) that would need 
to be fixed across the code base, I would propose that we roll it out on 
a per-directory basis. For example, we could do all of browser/ at the 
start of the 64 cycle just after the merges (alternately the end of the 
63 cycle during soft freeze), then do toolkit/ at the next merge time, 
and then pick up the rest at the end. This will limit the scope of 
possible bitrotting to smaller chunks, as well as making the patch sizes 
more manageable.


This will only cover directories that ESLint currently covers - though 
as we roll out ESLint to new directories, they'll be covered automatically.


Feedback & thoughts welcome.

Standard8

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to ship: application/json mime type support for OpenSearch Suggestions

[Mark Banner]

On 20/06/2018 22:14, Eric Shepherd (Sheppy) wrote:

Is there a bug number for this? I was not able to find one.


Sorry, I missed that: https://bugzilla.mozilla.org/show_bug.cgi?id=1425827

Mark.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Intent to ship: application/json mime type support for OpenSearch Suggestions

[Mark Banner]

In the draft OpenSearch specifications [1], there is ambiguity of the 
required mime type for providing suggestions for search providers.


Currently, the Suggestions extension [2] specifies 
"application/x-suggestions+json", however the example given in the main 
specification uses "application/json".


Since subtypes of json are typically no longer used, it seems reasonable 
to support both "application/x-suggestions+json" and "application/json".


Target Release: Firefox 63

Other browser support: Edge supports both, Chrome only supports 
"application/x-suggestions+json" at this time.



[1] 
https://github.com/dewitt/opensearch/blob/master/opensearch-1-1-draft-6.md#the-url-element


[2] 
http://www.opensearch.org/Specifications/OpenSearch/Extensions/Suggestions#URL_elements



___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Intent to Unship: support

[Mark Banner]

In bug 1452645 [1] we are planning to remove the "Load this bookmark in 
sidebar" feature. As part of this we will also be removing support for 
rel="sidebar" in anchor elements.


In the future, anchor elements with rel="sidebar" will be treated like 
normal anchor elements.


We are planning to make the change in Firefox 63.

From the specifications side, rel="sidebar" is only implemented by 
Firefox [2] and although it seems to have been mentioned in early 
versions of the HTML 5 specification, it isn't in the final version.


Extensions are able to open pages in the sidebar [3] (e.g. side view 
[4]), and intercept loads (e.g. see the Facebook Container add-on [5]), 
hence, they could add very similar functionality.


[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1452645

[2] https://developer.mozilla.org/docs/Web/HTML/Link_types

[3] 
https://developer.mozilla.org/Add-ons/WebExtensions/API/sidebarAction/setPanel


[4] https://testpilot.firefox.com/experiments/side-view

[5] https://addons.mozilla.org/firefox/addon/facebook-container/

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Intent to require Node 8.9.1/npm 5.5.1 for ESLint

[Mark Banner]

I would like to increase the minimum requirements for node with ESLint 
to node v8.9.1, npm v5.5.1 for the following reasons:


 * ESLint 5.x is now in alpha, and raises its minimum node requirement
   level to 6.14.0 (ours is currently 6.9.1)
 * MozillaBuild & our automation already use node 8.9.1
 * node 8.9.1 ships with npm 5.5.1
 * A lot has changed in npm between 3.10.x and 5.5.x, upgrading the
   minimum will provide better consistency for developers, especially
   with respect to npm-shrinkwrap.json/package-lock.json
 * This brings us closer to what was suggested in the "Intent to
   require Node to build..." thread.

I'm thinking about bumping this the week of 7th May - after the merges 
have completed.


I would like to hear feedback - positive or negative - from anyone 
likely to be affected by this proposal.


Please send comments to dev-platform or comment on bug 1456085. 



Thanks

Mark

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent To Require Manifests For Vendored Code In mozilla-central

[Mark Banner]

On 10/04/2018 05:25, glob wrote:
The plan is to create a YAML file for each library containing metadata 
such as the homepage url, vendored version, bugzilla component, etc. 
See https://goo.gl/QZyz4xfor the full specification.
There are some directories where we only import a file from a 
third-party and it is alongside other files in that same directory, e.g. 
testing/modules has "ajv-4.1.1.js" and "sinon-2.3.2.js" imported from 
elsewhere.


How do they fit into this scheme?

One thing we do need is a way for the build system to give us a list of 
files and directories that are from third-parties, imported, etc. Can 
that be provided as a result of this work?


Mark
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

PSA: Possible Issues running ESLint (could not load globals from file)

[Mark Banner]

We've had a few reports of issues when running ESLint in 
mozilla-central. The have tended to be along the lines of:


Could not load globals from file browser.js: Error: Invalid ecmaVersion.

If you get this, please check if you have a 
/tools/lint/eslint/node_modules/ directory, if so:


- remove the directory
- remove /node_modules/ as well.
- finally, run ./mach eslint --setup

You should then be able to run ESLint again.

(We believe this is a hang over from one of the early configurations we 
had in the tree that is now causing conflicts somehow).


Mark.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Most definitions of Ci, Cr, Cc, and Cu have been removed from the tree

[Mark Banner]

On 08/02/2018 08:56, Mark Banner wrote:

On 07/02/2018 00:38, Andrew McCreight wrote:

  Following up on my previous post, I have just landed (in bug 
1432992) the

removal of most of the definitions of Ci, Cr, Cc, and Cu. This touched
almost 2000 files, so it may affect your patches, but hopefully 
rebasing is

easy.
For those wanting an updated eslint-plugin-mozilla, please accept my 
apologies for the delay - I'm trying to publish at the moment, but for 
some reason the publishing isn't working. I've got a support request 
in with npm to try and get it resolved.
Pushing a new version (0.8.1) has been accepted on the npmjs repository, 
so others should be able to get the update now.


Mark.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Most definitions of Ci, Cr, Cc, and Cu have been removed from the tree

[Mark Banner]

On 07/02/2018 00:38, Andrew McCreight wrote:


  Following up on my previous post, I have just landed (in bug 1432992) the
removal of most of the definitions of Ci, Cr, Cc, and Cu. This touched
almost 2000 files, so it may affect your patches, but hopefully rebasing is
easy.
For those wanting an updated eslint-plugin-mozilla, please accept my 
apologies for the delay - I'm trying to publish at the moment, but for 
some reason the publishing isn't working. I've got a support request in 
with npm to try and get it resolved.


(I also thought that the Ci/Cr/Cc/Cu global definitions had already been 
published, but I obviously was wrong).


Mark.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: overly strict eslint rules

[Mark Banner]

On 03/01/2018 14:57, Mark Banner wrote:

On 24/12/2017 22:07, Masatoshi Kimura wrote:

---
$ mach lint browser/base/content/aboutDialog.js
eslint-plugin-html v2.0.3 should be v4.0.0.
ESLint is an old version, clobbering node_modules directory
Clobbering node_modules...
Installing eslint for mach using
"d:\mozilla-build\node-v8.1.4-win-x64\npm.cmd install 
--loglevel=error"...

npm ERR! code ENOLOCAL
npm ERR! Could not install from
"tools\lint\eslint\eslint-plugin-mozilla" as it does not contain a
package.json file.
In this case, I think it incorrectly removed the 
tools\lint\eslint\eslint-plugin-mozilla directory, if you check your 
source tree diffs, you should see if that was the case or not (though 
since this was a while ago, you may have already found that by now).


I've a feeling I know why, unfortunately my windows environment isn't 
very good at the moment, so I'll need to get that updated to investigate.
Just to follow up on this, this was a one-off issue due to a bad clobber 
when we attempted to update from v3 to v4 of ESLint. I've tracked down 
the issue <https://bugzilla.mozilla.org/show_bug.cgi?id=1428305>, and 
I'm near to a patch.


The workaround is running a revert on tools/lint/eslint/ and then 
running eslint again - though I suspect as we did the upgrade a while 
ago, most people will have already fixed this themselves.


Mark.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: overly strict eslint rules

[Mark Banner]

On 24/12/2017 22:07, Masatoshi Kimura wrote:

I got the following error when I tried it just now:
As far as I know, it should work fine with the latest version of 
MozillaBuild. If it doesn't, please file a bug in Testing:Lint and we'll 
take a look.

---
$ mach lint browser/base/content/aboutDialog.js
eslint-plugin-html v2.0.3 should be v4.0.0.
ESLint is an old version, clobbering node_modules directory
Clobbering node_modules...
Installing eslint for mach using
"d:\mozilla-build\node-v8.1.4-win-x64\npm.cmd install --loglevel=error"...
npm ERR! code ENOLOCAL
npm ERR! Could not install from
"tools\lint\eslint\eslint-plugin-mozilla" as it does not contain a
package.json file.
In this case, I think it incorrectly removed the 
tools\lint\eslint\eslint-plugin-mozilla directory, if you check your 
source tree diffs, you should see if that was the case or not (though 
since this was a while ago, you may have already found that by now).


I've a feeling I know why, unfortunately my windows environment isn't 
very good at the moment, so I'll need to get that updated to investigate.


Mark.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: overly strict eslint rules

[Mark Banner]

On 24/12/2017 19:41, Ben Kelly wrote:

But I also see rules about cosmetic things like what kind of quotes must be
used for strings.
AFAICT this kind of rule does not have any tangible safety benefit.  Its
purely a cosmetic style choice.  I don't see why we should bounce patches
out of the tree if the author and reviewer of a component prefer to use
single quotes instead of double quotes in a file.
As Jonathan already mentioned, the stylistic rules are designed to help 
enforce a consistent style, and reduce code review cycles to address 
review "nits" - benefiting both the submitter and reviewer.


This also helps newcomers find and pick up the consist style quickly, 
rather than having to "examine the files in the component and work out 
the style that's used most" which is what we've had in the past.


As a side note - the quotes rule was introduced at the request of 
Firefox peers to have consistent use of quotes across style (a 
consistent review nit, and frequently missed).

I also find this kind of cosmetic rule especially frustrating because we do
not run eslint by default.  AFAIK it does not even install with mach
bootstrap.  If we are going to enforce this kind of thing these checks
should be run as part of a build or when executing tests.  They should not
require manual installation of tools and running separate commands.
Thank you for highlighting this. Last year we were working on improving 
our hook support, with the intention of getting bootstrap hooked up as 
well. We didn't get time for that, so that will be the first priority 
this year.


I'm not yet convinced about hooking it into build / test execution. We 
could do this, but there's a variety of issues about when we run them 
and if we do/don't fail, that would make it harder in various instances 
(e.g. we have a rule to disallow 'debugger' statements, but you might 
want to run the tests with it). This might need a separate discussion.



As a further anecdote, I have also had to fight eslint rules in devtools
which required things like simultaneously using an extreme indent value and
not exceeding a line width limit.  Basically conflicting and impossible
rules to satisfy.  I ended up wasting close to an hour figuring out an ugly
work around for the problem.  It did not feel like the lint was providing
value.
devtools started out with stricter rules overall. For ESLint for most of 
mozilla-central, I think we have been conservative on the more stylistic 
rules. Note, they can be disabled in various sections if it makes sense 
(e.g. some of the accessible files disable no-multi-spaces, as it makes 
sense for those code blocks, but not for other places in the tree). I 
think longer term, we should consider changing the ESLint stylistic 
rules to using something like the "prettier" tool across the tree 
(similar to clang-format but for javascript - automatically formats).


Mark
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Proposal: Stop using Object.freeze/Object.seal on most of our Javascript Objects

[Mark Banner]

On 20/11/2017 13:22, Nicolas B. Pierron wrote:

On 11/20/2017 10:01 AM, Mark Banner wrote:
Now that we don't have legacy add-ons outside of our control, I would 
like to propose that we stop using Object.freeze/Object.seal on most 
of our objects.


I would personally recommend to every JavaScript developer that they 
should use these functions, in web pages, in JavaScript libraries, …


I believe that Object.freeze and Object.seal are sanity functions, 
part of defensive programming, used to guarantee that no other part of 
the code is doing something which is not intended.
I can understand the idea here, but I'm failing to believe that doing 
this would outweigh the benefits of having a unit test system where I 
can truly test at the unit level (or, more easily test at the 
integration/functional level).


If we had a complex function bar(), which, as part of it called 
Object.foo, how could I ensure that Object.foo is called with the 
correct parameters according to the inputs of bar()?


If Object is frozen, then I can't change Object, which means I either 
have to have some sort of abstraction interface, or a backdoor into 
Object, both of which would add extra overhead and complexity which 
doesn't seem to be the ideal.


Are there alternative ways we could achieve the same without the (or 
with low) complexity/overhead?


Mark.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Proposal: Stop using Object.freeze/Object.seal on most of our Javascript Objects

[Mark Banner]

[Resending to get the correct Firefox list]

Now that we don't have legacy add-ons outside of our control, I would 
like to propose that we stop using Object.freeze/Object.seal on most of 
our objects.


I believe that this was originally done to provide some extra 
security/stability relating to add-ons, i.e. stop them messing with our 
internal interfaces. Now that this isn't a problem, it seems that we 
should reconsider this.


One of the main reasons for opening this up would be to allow easier 
stubbing for unit/integration tests. For example, using sinon with 
stubbing makes it easier to work with - you don't have to cover multiple 
components for a "simple" test if you don't want to. Unfortunately, the 
current practice for using Object.freeze/Object.seal prevents this.


Are there any concerns or objections to changing this practice?

Mark.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Proposal: Stop using Object.freeze/Object.seal on most of our Javascript Objects

[Mark Banner]

Now that we don't have legacy add-ons outside of our control, I would 
like to propose that we stop using Object.freeze/Object.seal on most of 
our objects.


I believe that this was originally done to provide some extra 
security/stability relating to add-ons, i.e. stop them messing with our 
internal interfaces. Now that this isn't a problem, it seems that we 
should reconsider this.


One of the main reasons for opening this up would be to allow easier 
stubbing for unit/integration tests. For example, using sinon with 
stubbing makes it easier to work with - you don't have to cover multiple 
components for a "simple" test if you don't want to. Unfortunately, the 
current practice for using Object.freeze/Object.seal prevents this.


Are there any concerns or objections to changing this practice?

Mark.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

experimentalObjectRestSpread now turned on for ESLint

[Mark Banner]

We've just landed a patch in mozilla-central that turns on support in 
ESLint for Object Rest/Spread properties.


Javascript support for these landed in FF 55 (bug 1339395 
), and ESLint 
should now let you use them. More details on the properties can be found 
here:


https://github.com/tc39/proposal-object-rest-spread

Mark.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

PSA: ESlint 4 upgrade & node_modules clobber

[Mark Banner]

I have just landed an upgrade to ESLint 4 on autoland, this will make 
its way to mozilla-central later today.


One thing to note: on upgrading from ESLint 3 to 4, it will clobber your 
topsrcdir/node_modules directory - this is intentional to try and avoid 
issues we've seen previously with upgrades of npm modules (for npm 
version 5).


ESLint 4 has lots of new features, including glob based configuration 
options, and lots of improved rules. We'll be making more use of these 
over the next few cycles.


Please consider installing the lint commit/push hooks 
 
if you haven't already.


Mark


___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Experimenting with a shared review queue for Core::Build Config

[Mark Banner]

On 11/10/2017 19:41, Botond Ballo wrote:

On Wed, Oct 11, 2017 at 2:37 PM, Chris Cooper  wrote:

On Wed, Oct 11, 2017 at 1:46 PM, Nathan Froyd  wrote:

Does this user have a bugzilla :alias so that folks submitting patches
via MozReview or similar can just write r=build-peer or something,
rather than having to manually select the appropriate shared queue
after submitting their patch for review?

I see this as an added efficiency, so I'll see how much effort it is
to set this up. The people behind MozReview are explicitly not
expending effort there right now in favor of phabricator though.

It does not require any setup on the MozReview side - you just need to
add ":build-peer" (or whatever the chosen alias) to the "Real name"
field of the user's Bugzilla profile, and MozReview will autodetect
it.

In case anyone hits this, you need to add "r?Build" for mozreview to 
detect the reviewer properly.


Mark.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: We need better canaries for JS code

[Mark Banner]

On 19/10/2017 14:02, Ted Mielczarek wrote:


On Wed, Oct 18, 2017, at 07:51 AM, Mark Banner wrote:

Looping in firefox-dev as well, as I thin this is an important
discussion.

On 18/10/2017 09:28, David Teller wrote:

  Hi everyone,

Yesterday, Nightly was broken on Linux and MacOS because of a typo in
JS code [1]. If I understand correctly, this triggered the usual
"undefined is not a function", which was

1/ uncaught during testing, as these things often are;

Part of the reason it was uncaught, is that there's no automated test
coverage for that bit of code. It is a migration from one version to the
other, but unless there is an explicit test (and I don't see one) that
line won't be hit.

Given this bit, would any of the suggestions in this thread actually
help? If we're not exercising this code in automated tests, would "fail
tests on uncaught exceptions" make any difference?
The only thing that might help (that has been discussed) is something 
along the lines of flow - an analyser that could work out that 'spice()' 
didn't exist, but Dave Townsend mentioned these don't seem to be viable 
for us.


Therefore I think the migration code should be having automated tests - 
it is the only way we can reliably detect these sort of failures at the 
moment.


Mark.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: We need better canaries for JS code

[Mark Banner]

On 19/10/2017 03:29, Kris Maglione wrote:


On Wed, Oct 18, 2017 at 07:22:09PM -0700, Daniel Veditz wrote:
On Wed, Oct 18, 2017 at 4:51 AM, Mark Banner  
wrote:



I did an experiment, and the only way I got an error out was to have
"javascript.options.strict" on and



Why isn't it a code-style/review requirement that our own internal JS
include "use strict"? As a quick check I found 659 .jsm files in our 
tree

and only about 500 with "use strict". A quick skim of .js files shows a
similar ratio. It's not terrible (call it a "B" grade?) but we could do
better.


It is, in some areas, depending on their ESLint rules:

http://searchfox.org/mozilla-central/search?q=%22strict%22&case=true®exp=false&path=eslint 

Requiring "use strict" globally for ESLint is one of the items I've 
already had a few discussions about. The rough current plan is to get 
ESLint upgraded to version 4 (there's been a few blockers on that), then 
think about enabling more rules. Some of the rules are overlapping with 
what strict mode enforces, so I think we can turn those on earlier, then 
enable use strict slightly later.

And it's automatically enforced everywhere in JSM and JS component code.
To clarify, the "use strict" at the start of those files is just for 
show/to remind developers? Once we get ESLint 4, we can quite easily 
enforce showing "use strict" for all .jsm files.
To date, we've mostly avoided enforcing it for browser window code to 
avoid breaking extensions that used arguments.callee.caller (despite 
my endorsement for breaking those extensions...), but that's not 
really an issue anymore, so we should probably move towards enforcing 
it for all chrome JS.
Side note: fyi, we recently enabled the no-caller rule 
<https://bugzilla.mozilla.org/show_bug.cgi?id=1392119> for ESLint which 
stops this being used. We haven't got 100% ESLint coverage yet, but I 
believe we've covered most production code and it is mainly tests 
remaining (which we're working on 
<https://bugzilla.mozilla.org/show_bug.cgi?id=1357557>).


Mark
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: We need better canaries for JS code

[Mark Banner]

Looping in firefox-dev as well, as I thin this is an important discussion.

On 18/10/2017 09:28, David Teller wrote:

 Hi everyone,

   Yesterday, Nightly was broken on Linux and MacOS because of a typo in
JS code [1]. If I understand correctly, this triggered the usual
"undefined is not a function", which was

1/ uncaught during testing, as these things often are;
Part of the reason it was uncaught, is that there's no automated test 
coverage for that bit of code. It is a migration from one version to the 
other, but unless there is an explicit test (and I don't see one) that 
line won't be hit.


This seems to be a common theme with various migration routes in the 
code base, when I've asked about ensuring more automated testing there 
before I feel like I've got shrugs.


Admittedly, a lot of it is simple code, and should be correct, but as 
we've seen, mistakes can be made. Should we now be changing our policy?


At a minimum, I wonder if we could write some sort of simple test for 
CustomizableUI that would go through most routes (with minimal updates 
for each change).



2/ basically impossible to diagnose in the wild, because there was no
error message of any kind.
I did an experiment, and the only way I got an error out was to have 
"javascript.options.strict" on and "browser.dom.window.dump.enabled". 
Unfortunately the breakage was such that the browser console couldn't be 
opened, so you'd just having strict on wasn't enough.

I remember that we had bugs of this kind lurking for years in our
codebase, in code that was triggered daily and that everybody believed
to be tested.

I'd like to think that there is a better way to handle these bugs,
without waiting for them to explode in our user's face. Opening this
thread to see if we can find a way to somehow "solve" these bugs, either
by making them impossible, or by making them much easier to solve.
ESLint has caught some bugs - mainly undefined and unused related 
issues, and is spread through most of the production javascript code. 
Unfortunately it isn't able to catch this class of error. For that, we'd 
need something like Flow. Last time I looked at it, it didn't feel like 
a good fit for us, although I didn't go too deep, and I think there may 
have been other people that were looking at it.

I have one proposal. Could we change the behavior of the JS VM as follows?

- The changes affect only Nightly.
- The changes affect only mozilla chrome code (including system add-ons
but not user add-ons or test code).
- Any programmer error (e.g. SyntaxError) causes a crash a crash that
displays (and attaches to the CrashReporter) both the JS stack in and
the native stack.
- Any SyntaxError is a programmer error.
- Any TypeError is a programmer error.

I expect that this will find a number of lurking errorsy, so we may want
to migrate code progressively, using a directive, say "use strict
moz-platform" and static analysis to help encourage using this directive.
It would definitely be interesting to fail tests on some of the strict 
failures. I was surprised when I recently turned on the pref again to 
see how many warnings there were.


Having looked through a few of those, I've just found at least one issue 
, though 
non-critical, and I'm thinking we want to get the no-unused-expressions 
 rule turned on 
for ESLint as a result.


Mark
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

PSA: Missing instances of semicolon in js files landed on inbound

[Mark Banner]

We've just landed a patch to fix 
 
many instances of missing semicolons in js files onto mozilla-inbound. 
This are automatically fixed issues by ESLint.


Some 1156 files have been changed, so please be prepared to refresh your 
patches.



This work is in preparation to land 
 enabling of the 
Eslint rule 'semi'  sometime soon - 
hopefully later this week.


If you've not already set up hooks for Linting, you might want to do 
that soon. Instructions here: 
https://firefox-source-docs.mozilla.org/tools/lint/usage.html#using-a-vcs-hook


Mark.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

PSA: If using npm 5.0/5.0.1, please update to 5.1.0

[Mark Banner]

If you've got npm 5.0 or 5.0.1 installed, please update to 5.1.0 
. 
The latest version has a lot of fixes for its package handling when 
package.json is manually changed.


I'll be doing some ESLint dependency updates soon which might show up 
the issues if you have 5.0/5.0.1 installed.


Versions earlier than 5.0 shouldn't be affected.

Most npm installs can be updated with npm install -g npm

Mark

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

ESLint binary location change: Update your editors (after next merge)

[Mark Banner]

Continuing with the ESLint improvements, we've just landed a change 
 on autoland that 
moves some of the files for ESLint setup to the root directory of 
mozilla-central. It should merge later today if all goes well.


The change means that you should no longer need to tell your editor 
plugins for a specific location of the ESLint binary.


Previously you would have needed to specify 
|tools/lint/eslint/node_modules/.bin|, but no longer. All new developers 
need to do is install the appropriate plug-ins 
.


I've updated the devmo page  
to reflect these changes.


If you need help, feel free to ping me on irc if I'm about, or drop by 
#eslint.


Note: you can delete the old tools/lint/node_modules safely.

Mark

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

PSA: ESLint no-undef rule now enabled for all of toolkit/

[Mark Banner]

The "no undefined" eslint rule has now been enabled for all of toolkit/ 
- this landed last Friday.


The devmo  page has 
been updated with further information about the rule and some of the 
flags that are occasionally necessary.


Enabling it for all of browser/ will happen in the next few weeks.

Standard8
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

ESlint Updates

[Mark Banner]

Three quick notes about updates for things we're doing with ESlint and 
Firefox:


1. There's a new help page on devmo with ESLint tips & advice:
   https://developer.mozilla.org/docs/ESLint
2. Jared has been posting about about rules being enabled on his blog:
   https://msujaws.wordpress.com/
3. I have just landed enabling the no-undef
    rule for services/
    on autoland
   (assuming it sticks).

More information on no-undef:

 * I'm planning to enable it for toolkit/ and browser/ soon. This is
   likely to be a phased roll-out.
 * Due to the way our import system works (Cu.import and also xul/html
   files including multiple js files), detecting all the globals has
   been quite complicated. We're getting there, but it isn't perfect
   yet, but we've already been finding bugs and dead code, so the
   benefits are real.
 * The devmo page lists hints for working with no-undef
   . These should
   hopefully get simpler over time.

If there's any questions or comments, please stop by #eslint on irc.

Mark.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

PSA: Tail files now removed from xpcshell-tests

[Mark Banner]

We've just landed bug 503613 [1] that removes the tail file 
functionality from xpcshell tests.


Please use do_register_cleanup function for cleanup in future (most of 
our code had already been transitioned to/used this function).


Standard8

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=503613
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Strange error message on try-comm-central: Error: Invalid addon ID: expected addon ID specialpow...@mozilla.org, found special-pow...@mozilla.org in manifest

[Mark Banner]

On 23/09/2016 05:01, ishikawa wrote:

On 2016年09月23日 11:42, ishikawa wrote:


By changing a few install.rdf as suggested, the particular error messages
are gone, but
I now see different failures.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=de72a169065a2bbb7f69a0e5420992bfca5a77ec

I am testing other patches as well, the particular patch to modify
install.rdf is
https://hg.mozilla.org/try-comm-central/rev/a135ec33cb11

I will investigate more to see if the new errors are due to my changes or
a regression caused by a later change to M-C, etc.




The new errors are obviously related to my change.

...


inline void
CrashIfNotInAutomation()
{
MOZ_RELEASE_ASSERT(IsInAutomation());
}

So where is CrashIfNotInAutomation() called?


Did you look at the IsInAutomation() function?

https://dxr.mozilla.org/mozilla-central/rev/f0e6cc6360213ba21fd98c887b55fce5c680df68/js/xpconnect/src/xpcpublic.h#606

That's clearly checking a preference that is expected for automation. 
Looking at the tree, it gets set to true for most (probably all) of the 
tests that Firefox runs:


https://dxr.mozilla.org/mozilla-central/search?q=security.turn_off_all_security_so_that_viruses_can_take_over_this_computer&redirect=false

This probably means that the Thunderbird specific MozMill builders 
should either not be installing the special powers add-on (which I 
suspect is hard), or they should also be setting the pref to true.



I changed only these:


There's no need to include a full diff when you've already linked to it.

Mark.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Strange error message on try-comm-central: Error: Invalid addon ID: expected addon ID specialpow...@mozilla.org, found special-pow...@mozilla.org in manifest

[Mark Banner]

On 22/09/2016 08:44, ISHIKAWA,chiaki wrote:

3:07:56 INFO -  1474488476446addons.xpi-utilsWARN
addMetadata: Add-on specialpow...@mozilla.org is invalid: Error: Invalid
addon ID: expected addon ID specialpow...@mozilla.org, found
special-pow...@mozilla.org in manifest  ...


I am not sure if this comes from the intended test.
Rather it looks a configuration error.
The error line seems to appear at beginning of the selected run of each
test directory.


...

cf. 3 results from the comm-central tree:
mozilla/testing/specialpowers/Makefile.in
♢7 XPI_PKGNAME = specialpow...@mozilla.org


Looks like this is different from what is specified in 
testing/specialpowers/install.rdf - which is the dashed version, and was 
introduced when it was rewritten last year.


The simplest thing is probably to change the install.rdf to remove the 
dash as then it wouldn't be changing the file name on disk - and also 
update the other couple of instances in the tree. Probably worth a 
running through a few tests on try just to make sure as well.


The test infrastructure has probably got away with it due to the next 
warning of "Could not uninstall invalid item from locked install 
location", so it kept on using it.


Mark
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Firefox Hello new data collection

[Mark Banner]

On 04/04/2016 16:49, a...@imgland.xyz wrote:

I don't know much about Mozilla's privacy but in my opinion feel the
need to immediately remove it from Firefox and push a new beta build


I can understand your concern, however, please understand that this 
logging functionality is currently disabled by default - see the 
"loop.logDomains" preference.


We won't be enabling it until the privacy review is completed.

If you wish to inspect and validate my assertion, you are quite welcome to.

Here's a link to the code so that you can see what is currently on beta

http://mxr.mozilla.org/mozilla-beta/search?string=loop.logDomains&find=&findi=&filter=%5E%5B%5E%5C0%5D*%24&hitlimit=&tree=mozilla-beta

You can also see from the test file there, that we have a test to check 
that nothing is logged if the pref is set to false.


Mark.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Too many oranges!

[Mark Banner]

On 22/12/2015 20:41, Douglas Turner wrote:

My suggest was to take the n-th milestone was a strawman. I think in terms
of scheduling, setting aside every n-th milestone as a quality release
helps planning a bunch.  When we tell customers (the web, the firefox team,
firefoxos) that feature X will be delivered by Aug, we can build in time to
sharpen our axe.


On Hello, we've being attempting to take care of quality slightly 
different. We're working in an agile manner, and for each cycle, we're 
trying to allocate a certain amount of time to fixing bugs, addressing 
technical debt, and other quality issues. I can't remember the exact 
percentages at the moment, but I know we originally discussed something 
like 20%. The remaining 80% is spent on feature work. We're also 
starting to expect/allocate more time for fully implementing features 
after the "initial" implementation is complete.


We've only been trying this over the last couple of months, and with 
other changes going on at the same time, I think its too early to say 
how successful we're being. For me, the positive part, is that we're 
constantly thinking about quality and improving the code base, rather 
than doing it in spurts when it gets very bad.


Mark.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Voting in BMO

[Mark Banner]

On 09/06/2015 23:19, R Kent James wrote:

Without voting, how do you direct users to express an interest in seeing
a bug solved without adding a "me too" comment?


Maybe the answer is that we don't. We just need to be honest - it would 
be very difficult to create a fair system that could be used in a valid way.


Allowing votes by users that then don't actually get looked at or 
considered, is just misleading the user who's taken the time to create 
an account, sign-in and then vote.


Replying to this got me thinking - what about changing "vote" to "I have 
this issue" similar to support.mozilla.org. However, I think the 
usefulness would end up in the same way as voting.


As a suggestion though, how about adding something near the comment box 
- like a checkbox or button which says "I have this issue and wish to 
receive updates about it". If it was a checkbox, it could disable the 
comment box.


I'm not sure if that could also be misleading, but the intention would 
be to add an option for a user to give them something other than just 
the comment box - which at the moment, is the only real obvious 
"associate me" with this bug option.


Mark.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: What are your pain points when running unittests?

[Mark Banner]

On 13/03/2015 20:02, Gregory Szorc wrote:

Another pain point: running all relevant tests.


Agreed, something we've done on Loop/Hello as an intermediate step is to 
have a shell file which knows about all the likely relevant tests and 
runs them all:


http://mxr.mozilla.org/mozilla-central/source/browser/components/loop/run-all-loop-tests.sh

This at least lets developers run those ones locally.

Mark
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: ES6 lexical temporal dead zone has landed on central

[Mark Banner]

On 16/09/2014 09:09, Philip Chee wrote:

On 16/09/2014 07:43, Shu-yu Guo wrote:

Hello all,



Everything inside mozilla-central needed to make the tree green on TBPL has
been fixed. However, this is expected to break addons, comm-central, and
possibly B2G code that use `let` (though I was told last time that Gaia does
not use `let`).


Hello!

In future please remember to file appropriate bugs in comm-central
*before* landing such tree-wide changes. Your cooperation is appreciated.


Sorry, but I have to correct you. This is NOT a requirement on 
gecko/Firefox developers.


If we can get notification (via cc, this list, other lists or via bugs) 
of big, potentially-breaking changes, before they land, that is 
appreciated and is useful to us as it helps save time. However, we 
recognise this isn't always possible/practical (or remembered).


It is the responsibility of comm-central maintainers/developers to 
ensure any necessary bug filing takes place. This doesn't stop 
gecko/Firefox developers from filing bugs and helping us out, but it 
certainly isn't a requirement.


Mark.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Upcoming changes to autocomplete code affecting Thunderbird

[Mark Banner]

On 15/08/2014 12:55, Paolo Amadini wrote:

The major change would be the removal of the multiple search sources
from the autocomplete code, since this feature is used heavily by
Thunderbird. For Firefox, we already developed the UnifiedComplete
component that replaces the multiple searches in the location bar, but a
similar work would be required for Thunderbird.


So if I understand you right, we could switch to UnifiedComplete if we 
want to maintain multiple search sources (whilst using toolkit code)?




*Proposal*

Since I believe the amount of work and testing required in Thunderbird
to keep up with the changes could be non-trivial, my suggestion would
be to fork the current version of the autocomplete code from Toolkit to
the comm-central repository. The autocomplete code itself does not
depend on other components, and "pinning" the current version in
Thunderbird would make it easier to keep up with the planned rapid
development. Any wanted changes could then be imported as needed.


That sounds like a reasonable plan. Its kinda ironic that we've recently 
moved Thunderbird off the xpfe version to the toolkit one, but can't be 
helped.


In the meantime it keeps us stable - there's potentially possibilities 
what we do, switch to UnifiedComplete or rewrite the address book (which 
has been on the plans for a while now).


Mark.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Proposal: Move Thunderbird and SeaMonkey to mozilla-central

[Mark Banner]

[follow-ups to mozilla.dev.planning]

tl;dr We're proposing to move Thunderbird and SeaMonkey into
mozilla-central to reduce maintenance complexities for build systems,
and for releng. Multiple-app handling in the same repository has
improved greatly, and the general rules would remain the same.

Feedback and discussion is most welcome - we have generally discussed
this within some individuals, but not widely.


Proposal


We'd like to move Thunderbird and SeaMonkey (i.e. most of the contents
of comm-central) into mozilla-central.

Schedule:

We're looking to complete this in the Gecko 31 cycle, as then we'd be 
able to pick it up in the next ESR cycle.


We already have this in preview mode running on the Alder twig.

Reasons:

There are several primary pain points that make things more complicated
for Thunderbird development, and its support for releases from releng.
The overarching reason is that this merge would reduce the amount of
work required to keep Thunderbird building to support security releases
and community development, and allow more focus on other areas.

They reasons we'd like to do this are:

- Requiring that comm-central pulls mozilla-central duplicates the build
system in weird and wonderful ways.

Maintaining the duplicate build system obviously duplicates much of the
work that is already being done in mozilla-central.

- Additionally, it would simplify pieces of m-c build config logic which 
are accommodating c-c.



- Due to pulling different repositories, the workarounds for releng are
complex. It requires an almost totally-separate project configuration,
and when build configurations change, these must be manually ported to
the Thunderbird project configuration.

With a similar configuration to Firefox, we could base on the same
configuration with only a few changes to project name, and specific
files etc.


- Additionally, some benefits from search&replace patches are lost as
the code is in a separate repository.

Impacts
---

History:

The merge will include the full c-c history.

Rules:

We'd obviously keep the same rules - changes that break
Thunderbird/SeaMonkey are not required to be fixed by the Gecko change
that breaks them.

We may relax some of the existing comm-central rules, e.g. Gecko build
system review could apply to the Thunderbird parts for search & replace
patches, rather than requesting separate reviews. Obviously, we'd
confirm with the appropriate areas before this happens.

Build:

Now that bug 787449 [1] is fixed, we can easily exclude 
Thunderbird/SeaMonkey pushes from triggering gecko builds.


Tree:

We envision that the existing Thunderbird-Trunk would be kept separate
to the Mozilla-Central one, however, we could consider merging these at
a later date if it makes sense.

Directories:

The additional directories would currently be approximately:

mozilla-central/mail
mozilla-central/mailnews
mozilla-central/calendar
mozilla-central/chat
mozilla-central/im
mozilla-central/suite
mozilla-central/editor/ui
mozilla-central/db/mork
mozilla-central/other-licenses/branding/thunderbird
mozilla-central/ldap (also statically importing the ldap c-sdk).

However, we would be willing to restructure/group those if they are too
much/complex.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=787449
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: What platform features can we kill?

[Mark Banner]

Maybe not quite platform features, but on the subject of removing or js
replacements, I offer up a couple of candidates:

http://mxr.mozilla.org/comm-central/source/mozilla/xpfe/components/directory/

I believe this is an rdf datasource for listing ftp directory pages.
AFAIK this might only be used by SeaMonkey now, so could be moved out.

http://mxr.mozilla.org/comm-central/source/mozilla/xpfe/components/windowds/

This appears only used in a couple of places, but I don't know enough
about it to fully make an assessment, but its pretty old code and rdf
based as well.

Mark.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: review stop-energy (was 24hour review)

[Mark Banner]

On 11/07/2013 12:59, Gervase Markham wrote:
> On 09/07/13 21:29, Chris Peterson wrote:
>> I've seen people change their Bugzilla name to include a comment about
>> being on PTO. We should promote this practice. We could also add a
>> Bugzilla feature (just a simple check box or a PTO date range) that
>> appends some vacation message to your Bugzilla name.
> 
> Hey, if we had a PTO app that tracked all absences, we could integrate
> with it...
> 

I'd rather have a custom field anyway. Sometimes I'm highly focussed on
a project for particular reason, or doing meet-up weeks. During those
times I rarely even check bugmail for my other projects.

Mark.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: review stop-energy (was 24hour review)

[Mark Banner]

On 09/07/2013 21:29, Chris Peterson wrote:
>> I really
>> wish Bugzilla could let me flag myself as not available for reviews when
>> I'm on vacation, say.  Expecting people to comment about being on
>> vacation while on vacation is, imo, not reasonable.
> 
> I've seen people change their Bugzilla name to include a comment about
> being on PTO. We should promote this practice. We could also add a
> Bugzilla feature (just a simple check box or a PTO date range) that
> appends some vacation message to your Bugzilla name.

The problem is, that doesn't work on the patch submission forms. If
bugzilla does decide to autocomplete, you can't necessarily see all the
info on the name, because the field isn't wide enough.

If you don't need/look at the autocomplete, then you get zero indication
of the  bugzilla name.

The only real chance you get to see that name change is if you're on a
bug where they have already commented or filed.

I wouldn't want something just for PTOs (e.g. meetups can take up review
time for projects I'm not meeting up for), but I think it would be good
to have the option to provide a warning with a little bit of text *and*
have that a) displayed on the submission forms, and b)
prompted/confirmed if the user decides to request anyway.

Mark.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Bug 851818 - Modernizing the Enter Bug Entry Page in Bugzilla

[Mark Banner]

On 18/03/2013 16:27, Jason Smith wrote:

- BootToGecko - Big focus at Mozilla with a lot of people working on
this project, so I think this could benefit greatly from being on the
front page.


This should be Firefox OS as that's what the users will know it as.


*Rationale for Removals*

- Thunderbird - Decreased focus at Mozilla now that has moved to
contribution only now, so I don't think this needs to be on the front page.


Mozilla is still committed to providing security and stability updates, 
as well as enabling those contributions. We need the bug reports to 
enable this to happen, and I think making Thunderbird harder to find 
would send the wrong message here.


Mark.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Gecko 17 as the base for B2G v1

[Mark Banner]

On 01/08/2012 22:47, Alex Keybl wrote:

On 10/7, we'll similarly move active B2G v1 platform development to
mozilla-beta, and on 11/19 work will move to the mozilla-esr17 branch. A
notice referencing this dev.platform post will be sent to the enterprise
list shortly for comment, as some of the ESR goals in [1] will need to
change slightly.


Can you elaborate on the goal changes?

The only thing I can see from your enterprise post is the addition of 
the support of B2G off the next ESR branch.


If I'm understanding correctly then there is no intended affect on ESR 
itself?


Thanks
Mark.


[1] https://wiki.mozilla.org/Enterprise/Firefox/ExtendedSupport:Proposal


___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Mac OS X 10.5 Support Plans (Updated)

[Mark Banner]

On 19/07/2012 21:38, Alex Keybl wrote:

Thanks to everyone who joined this discussion. We've now moved
forward with disabling OS X 10.5 support for FF17 in [1]. Per
discussion here, we will strive to leave code in place through the 17
cycle, but if it becomes difficult to accomplish necessary work, we
may end up breaking things to advance our goals for supported Mac
versions.


Will we also be stopping the universal binaries? AIUI the 32 bit part 
was only kept around for the 10.5 support, and running 32 bit on 10.6 is 
unsupported.


Hence I don't think we need the 32 bit part any more?

If we do drop that, then I believe that'd be a significant reduction on 
our build & release systems...


Mark.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform