Re: Intent to unship: XUL grid implementation

[Brian Grinstead]

This is wonderful - it helps the layout engine focus on web standard
implementations and gets rid of some quirky behavior in chrome UI. Thanks
for your persistence Tim.

Brian

On Mon, Nov 16, 2020 at 5:18 PM Tim Nguyen  wrote:

> Hi folks,
>
> Thanks to Mats Palmgren & Rares Doghi, XUL grid got removed from tab
> prompts
> 
> (`alert`/`confirm`/`prompt`) last week, this was the last usage in
> mozilla-central. If you notice any regressions, please file a bug and mark
> it as a regression of bug 1583696
> .
>
> All other XUL grid removals have baked for a year now.
>
> With that being said, I'm planning to unship the XUL grid implementation in
> Firefox 85 in bug 1525737
> , which includes:
>
>- the whole layout/xul/grid directory
>
>- the -moz-grid/-moz-grid-group/-moz-grid-line CSS display values
>- the // XUL elements
>
> If you need grid-like functionality, alternatives are CSS grid or HTML
> tables.
> Please let me know if you have any concerns or questions.
>
> Cheers,
> Tim
> ___
> 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

Re: A new testing policy for code landing in mozilla-central

[Brian Grinstead]

Hi Jeff,
Thanks to help from Marco Castelluccio we now have an artifact in taskcluster 
that has metadata about all commits that land in m-c, which I've used to gather 
this data. We're still working on tooling to better understand patterns with 
different types of bugs & patches, but I'm happy to share what we already have.

Here's a spreadsheet showing weekly data since September: 
https://docs.google.com/spreadsheets/d/1xpm7V-zHCB3nyENdoVkG2ViT5iyg940SIpobY1fzR2g/edit.

A few notes:

* "none" is when a patch is reviewed/landed without a tag being applied. I've 
been hoping this would naturally get down to 0 as people get used to the policy 
and from the notifications we added in phabrictor. It dropped quickly to ~10% 
but is hovering right now. I've spoken with the Lando team about changing our 
warning checkbox into a hard blocker to enforce this.
* "unknown" counts revisions that don't have an accessible phabricator 
revision. This is mostly automated commits like wpt-sync (for example 
https://hg.mozilla.org/mozilla-central/rev/19b1604e8c8e) but also includes 
revisions with restricted permissions that aren't able to be loaded during 
generation time. For the sake of reporting I think these can be pretty much 
ignored.
* From most common to least common we see: “unchanged”, “approved”, “other”, 
“elsewhere”, “ui”. This has remained pretty stable across weeks. I’m not 
surprised that “unchanged” is common since it’s a bit overloaded - covering 
both (a) code that don’t ship to users and (b) code that’s being modified but 
keeping the same functionality.

Finally, I’m happy to hear feedback about how people think this has been going 
from a reviewer/author standpoint. Most of the feedback has been around 
ambiguity with “unchanged”. For example when “unchanged” versus “approved” 
should be applied when touching only tests: 
https://bugzilla.mozilla.org/show_bug.cgi?id=1672439. Or if we should somehow 
split up “unchanged” to differentiate between code that already has tests and 
code that doesn’t. #testing-policy is a good channel for that discussion if you 
are interested in the topic.

Thanks,
Brian

> On Nov 12, 2020, at 8:21 AM, Jeff Muizelaar  wrote:
> 
> Brian,
> 
> Now that we've been doing this for a while, do we have any aggregated
> data on the testing tags? i.e. how often are we adding tests vs not.
> 
> -Jeff
> 
> On Tue, Sep 15, 2020 at 11:03 AM Brian Grinstead  
> wrote:
>> 
>> (This is a crosspost from firefox-dev)
>> 
>> Hi all,
>> 
>> We’re rolling out a change to the review process to put more focus on 
>> automated testing. This will affect you if you review code that lands in 
>> mozilla-central.
>> 
>> ## TLDR
>> 
>> Reviewers will now need to add a testing Project Tag in Phabricator when 
>> Accepting a revision. This can be done with the “Add Action” → “Change 
>> Project Tags” UI in Phabricator. There's a screenshot of this UI at 
>> https://groups.google.com/g/firefox-dev/c/IlHNKOKknwU/m/zl6cOlT2AgAJ.
>> 
>> We’ve been piloting the policy for a few weeks with positive feedback from 
>> reviewers. Still, there may be some rough edges as we roll this out more 
>> widely. I’d like to hear about those, so please contact me directly or in 
>> the #testing-policy channel in Slack / Matrix if you have feedback or 
>> questions about how the policy should apply to a particular review.
>> 
>> We’re working on ways to make this more convenient in the UI and to prevent 
>> landing code without a tag in Lando. In the meantime if you'd like a 
>> reminder to add the tag while reviewing code, Nicolas Chevobbe has built a 
>> WebExtension that automatically opens up the Project Tags UI when 
>> appropriate at https://github.com/nchevobbe/phab-test-policy.
>> 
>> ## Why?
>> 
>> Automated tests in Firefox and Gecko reduce risk and allow us to quickly and 
>> confidently improve our products.
>> 
>> While there’s a general understanding that changes should have tests, this 
>> hasn’t been tracked or enforced consistently. We think defining an explicit 
>> policy for including automated tests with code changes will help to achieve 
>> those goals.
>> 
>> Also, we’ll be able to better understand exactly why and when tests aren’t 
>> being added. This can help to highlight components that need new testing 
>> capabilities or technical refactoring, and features that require increased 
>> manual testing.
>> 
>> There are of course trade-offs to enforcing a testing policy. Tests take 
>> time to write, maintain, and run which can slow down day-to-day development. 
>> And given the complexity of a modern web 

Re: A new testing policy for code landing in mozilla-central

[Brian Grinstead]

> On Sep 24, 2020, at 7:02 AM, Kartikaya Gupta  wrote:
> 
> First - thank you for making these changes! I do find the testing
> prompt useful as a reviewer, and it has already resulted in a few
> extra tests being added where they probably wouldn't have been
> otherwise.
> 
> After living with this for a week or so, I have two (relatively minor)
> papercuts:

Thanks for the feedback:

> - I really want the webextension available in Fenix. I do a surprising
> number of reviews from my phone and having to manually add the tags is
> annoying

A shorter term goal would be to ship a Phabricator extension that implements 
the web extension behavior so you don’t need to install anything. Longer term 
it'd be great if the UI more integrated into the Accept process rather than 
being driven through the Project Tags UI (for example, a radio list to select 
from before you can click Submit).

Of course getting a Phabricator extension rolled out for everyone has a much 
higher bar for testing and quality compared with the web extension, and needs 
to be balanced with everything else on that team's roadmap. But it's good 
motivation to hear you are finding the web extension useful.

> - Sometimes I want to mark a patch "testing-exception-elsewhere" or
> "testing-exception-other" but also leave a general review comment. It
> seems awkward to me to have to mix my general review comments with the
> testing-specific comment into the same input field, and I'm always
> worried it will confuse the patch author. If this process is
> eventually formalized into something more structured than phabricator
> project tags it would be nice to have separate fields for the
> testing-related comment and the general review comment.

Yeah, keeping a structured field with the testing policy comment would also be 
great for querying or scanning revisions. Maybe something in the revision 
details - though I’d want the ability to comment more integrated than having to 
go to Edit Revision as a separate step _after_ adding the Project Tag. I've 
pinged Zeid to ask if there's any existing functionality along these lines. 

> Cheers,
> kats
> 
> On Tue, Sep 15, 2020 at 11:03 AM Brian Grinstead  
> wrote:
>> 
>> (This is a crosspost from firefox-dev)
>> 
>> Hi all,
>> 
>> We’re rolling out a change to the review process to put more focus on 
>> automated testing. This will affect you if you review code that lands in 
>> mozilla-central.
>> 
>> ## TLDR
>> 
>> Reviewers will now need to add a testing Project Tag in Phabricator when 
>> Accepting a revision. This can be done with the “Add Action” → “Change 
>> Project Tags” UI in Phabricator. There's a screenshot of this UI at 
>> https://groups.google.com/g/firefox-dev/c/IlHNKOKknwU/m/zl6cOlT2AgAJ.
>> 
>> We’ve been piloting the policy for a few weeks with positive feedback from 
>> reviewers. Still, there may be some rough edges as we roll this out more 
>> widely. I’d like to hear about those, so please contact me directly or in 
>> the #testing-policy channel in Slack / Matrix if you have feedback or 
>> questions about how the policy should apply to a particular review.
>> 
>> We’re working on ways to make this more convenient in the UI and to prevent 
>> landing code without a tag in Lando. In the meantime if you'd like a 
>> reminder to add the tag while reviewing code, Nicolas Chevobbe has built a 
>> WebExtension that automatically opens up the Project Tags UI when 
>> appropriate at https://github.com/nchevobbe/phab-test-policy.
>> 
>> ## Why?
>> 
>> Automated tests in Firefox and Gecko reduce risk and allow us to quickly and 
>> confidently improve our products.
>> 
>> While there’s a general understanding that changes should have tests, this 
>> hasn’t been tracked or enforced consistently. We think defining an explicit 
>> policy for including automated tests with code changes will help to achieve 
>> those goals.
>> 
>> Also, we’ll be able to better understand exactly why and when tests aren’t 
>> being added. This can help to highlight components that need new testing 
>> capabilities or technical refactoring, and features that require increased 
>> manual testing.
>> 
>> There are of course trade-offs to enforcing a testing policy. Tests take 
>> time to write, maintain, and run which can slow down day-to-day development. 
>> And given the complexity of a modern web browser, some components are 
>> impractical to test today (for example, components that interact with 
>> external hardware and software).
>> 
>> The policy below hopes to mitigate 

Re: A new testing policy for code landing in mozilla-central

[Brian Grinstead]

> On Sep 16, 2020, at 7:12 AM, surkov.a...@gmail.com 
>  wrote:
> 
> Better test coverage is the best. Agreed, having a policy to force developers 
> to add tests must be an efficient way to improve test coverage. I worried 
> though it may reduce amount of contributions from the community since it 
> complicates the development process: sometimes you have to spend more time to 
> create a test than to fix a bug.

I think this is similar to the issue raised by mccr8 and discussed upthread. 
People who aren't familiar with a particular area of the codebase should still 
feel free to write a patch without a test to fix a bug. It's then up to the 
reviewer to either provide instructions to write a test, write a test 
themselves, or grant an exception. 

> Also it could make developers (intentionally or unintentionally) to pick up 
> tasks that don't need tests. The worse thing is perhaps you cannot 
> effectively measure these side affects of the change.

The exceptions are meant to at least partially address these points (see also 
ekr's point (2) above).

Brian

> Thanks,
> Alexander.
> 
> On Tuesday, September 15, 2020 at 11:03:45 AM UTC-4, Brian Grinstead wrote:
>> (This is a crosspost from firefox-dev) 
>> 
>> Hi all, 
>> 
>> We’re rolling out a change to the review process to put more focus on 
>> automated testing. This will affect you if you review code that lands in 
>> mozilla-central. 
>> 
>> ## TLDR 
>> 
>> Reviewers will now need to add a testing Project Tag in Phabricator when 
>> Accepting a revision. This can be done with the “Add Action” → “Change 
>> Project Tags” UI in Phabricator. There's a screenshot of this UI at 
>> https://groups.google.com/g/firefox-dev/c/IlHNKOKknwU/m/zl6cOlT2AgAJ. 
>> 
>> We’ve been piloting the policy for a few weeks with positive feedback from 
>> reviewers. Still, there may be some rough edges as we roll this out more 
>> widely. I’d like to hear about those, so please contact me directly or in 
>> the #testing-policy channel in Slack / Matrix if you have feedback or 
>> questions about how the policy should apply to a particular review. 
>> 
>> We’re working on ways to make this more convenient in the UI and to prevent 
>> landing code without a tag in Lando. In the meantime if you'd like a 
>> reminder to add the tag while reviewing code, Nicolas Chevobbe has built a 
>> WebExtension that automatically opens up the Project Tags UI when 
>> appropriate at https://github.com/nchevobbe/phab-test-policy. 
>> 
>> ## Why? 
>> 
>> Automated tests in Firefox and Gecko reduce risk and allow us to quickly and 
>> confidently improve our products. 
>> 
>> While there’s a general understanding that changes should have tests, this 
>> hasn’t been tracked or enforced consistently. We think defining an explicit 
>> policy for including automated tests with code changes will help to achieve 
>> those goals. 
>> 
>> Also, we’ll be able to better understand exactly why and when tests aren’t 
>> being added. This can help to highlight components that need new testing 
>> capabilities or technical refactoring, and features that require increased 
>> manual testing. 
>> 
>> There are of course trade-offs to enforcing a testing policy. Tests take 
>> time to write, maintain, and run which can slow down day-to-day development. 
>> And given the complexity of a modern web browser, some components are 
>> impractical to test today (for example, components that interact with 
>> external hardware and software). 
>> 
>> The policy below hopes to mitigate those by using a lightweight enforcement 
>> mechanism and the ability to exempt changes at the discretion of the code 
>> reviewer. 
>> 
>> ## Policy (This text is also located at 
>> https://firefox-source-docs.mozilla.org/testing/testing-policy/) 
>> 
>> Everything that lands in mozilla-central includes automated tests by 
>> default. Every commit has tests that cover every major piece of 
>> functionality and expected input conditions. 
>> 
>> One of the following Project Tags must be applied in Phabricator before 
>> landing, at the discretion of the reviewer: 
>> 
>> * `testing-approved` if it has sufficient automated test coverage. 
>> * One of `testing-exception-*` if not. After speaking with many teams across 
>> the project we’ve identified the most common exceptions, which are detailed 
>> below. 
>> 
>> ### Exceptions 
>> 
>> * testing-exception-unchanged: Commits that don’t change behavior for end 
>> users. For example: 
>> 

Re: A new testing policy for code landing in mozilla-central

[Brian Grinstead]

> On Sep 15, 2020, at 11:44 AM, Andrew Halberstadt  wrote:
> 
> On Tue, Sep 15, 2020 at 12:28 PM Andrew McCreight 
> wrote:
>> I don't know that tests being an official requirement relieves the burden
> of guilt of asking for tests, as everybody already knows that tests are
> good and that you should always write tests
> 
> I think this is largely true at Mozilla, but we all have our lapses from
> time to time. Thankfully since we all know this, a tiny nudge is all that
> we need.
> 
>> Separately, one category of fixes I often deal with is fixing leaks or
> crashes in areas of the code I am unfamiliar with. I can often figure out
> the localized condition that causes the problem and correct that, but I
> don't really know anything about, say, service workers or networking to
> write a test. Do I need to hold off on landing until I can find somebody
> who is willing and able to write a test for me, or until I'm willing to
> invest the effort to become knowledgeable enough in an area of code I'm
> unlikely to ever look at again? Neither of those feel great to me.
> 
> This sounds like an argument in favour of putting the burden of exceptions
> on the reviewer. Authors can submit without a test (ideally calling out
> it's because they don't know how) and then the reviewer can either:
> A) Explain how to add tests for said patch, or
> B) Knowing that writing a test would be difficult, grant them an exception

Or (C) the reviewer could write the test themselves and land it in a separate 
commit alongside the patch, marking the original patch as 
`testing-exception-elsewhere`.

We definitely don't want to discourage writing drive-by / “good samaritan” 
patches where you are fixing a bug in an area outside of your normal work. This 
came up a few times in discussions and is one of the reasons I prefer 
reviewer-specified instead of author-specified exceptions.

> If a reviewer finds themselves explaining / granting exceptions often, it
> could be a signal that the test infrastructure in that area needs
> improvement.
> Btw I think your points are all valid, I guess we'll have to see how it
> turns out in practice. Hopefully we can adjust the process as needed based
> on what we learn from it.

For sure. I’m happy to discuss any specific pain points you run into with the 
policy.

> - Andrew
> 
> On Tue, Sep 15, 2020 at 12:28 PM Andrew McCreight 
> wrote:
> 
>> On Tue, Sep 15, 2020 at 8:03 AM Brian Grinstead 
>> wrote:
>> 
>>> (This is a crosspost from firefox-dev)
>>> 
>>> Hi all,
>>> 
>>> We’re rolling out a change to the review process to put more focus on
>>> automated testing. This will affect you if you review code that lands in
>>> mozilla-central.
>>> 
>>> ## TLDR
>>> 
>>> Reviewers will now need to add a testing Project Tag in Phabricator when
>>> Accepting a revision. This can be done with the “Add Action” → “Change
>>> Project Tags” UI in Phabricator. There's a screenshot of this UI at
>>> https://groups.google.com/g/firefox-dev/c/IlHNKOKknwU/m/zl6cOlT2AgAJ.
>>> 
>> 
>> I of course think having more tests is a good thing, but I don't like how
>> this specific process places all of the burden of understanding and
>> documenting some taxonomy of exceptions on the reviewer. Reviewing is
>> already a fairly thankless and time consuming task. It seems like the way
>> this is set up is due to the specifics of our how reviewing process is
>> implemented, so maybe there's no way around it.
>> 
>> Also, contrary to what ahal said, I don't know that tests being an official
>> requirement relieves the burden of guilt of asking for tests, as everybody
>> already knows that tests are good and that you should always write tests.
>> The situation is different from the linters, as the linters will fix almost
>> all issues automatically, so asking somebody to fix linter issues isn't
>> much of a burden at all. I guess it is impossible to make testing better
>> without somebody directly pressuring somebody, though.
>> 
>> My perspective might be a little distorted as I think a lot of the patches
>> I write would fall under the exceptions, either because they are
>> refactoring, or I am fixing a crash or security issue based on just a stack
>> trace.
>> 
>> Separately, one category of fixes I often deal with is fixing leaks or
>> crashes in areas of the code I am unfamiliar with. I can often figure out
>> the localized condition that causes the problem and correct that, but I
>> don't really know anything about, say, service worker

Re: A new testing policy for code landing in mozilla-central

[Brian Grinstead]

> On Sep 15, 2020, at 8:48 AM, Jonathan Kew  wrote:
> 
> On 15/09/2020 16:03, Brian Grinstead wrote:
> 
> > We’re rolling out a change to the review process to put more focus on 
> > automated testing. [...]
> 
> As others have said, it's great that we're setting a clearer policy here - 
> thanks!
> 
> One thing did strike me as a little surprising, and could perhaps be 
> clarified:
> 
> > * testing-exception-other: Commits where none of the defined exceptions 
> > above apply but it should still be landed. This should be scrutinized by 
> > the reviewer before using it - consider whether an exception is actually 
> > required or if a test could be reasonably added before using it. This 
> > requires a comment from the reviewer explaining why it’s appropriate to 
> > land without tests.
> 
> The point that caught my eye here was that it explicitly requires the 
> explanatory comment to be *from the reviewer*. I would have thought that in 
> many cases it would make sense for the *patch author* to provide such an 
> explanatory comment (which the reviewer would of course be expected to 
> scrutinize before granting r+). Is that scenario going to be considered 
> unacceptable?
> 

I think that’s OK and something we can clarify in the text if needed. It’s 
helpful for authors to explain the reasoning if they think an exception should 
be applied (essentially, making a request for a particular exception). We'd 
need to decide if either (a) the reviewer grants the request by adding the 
exception with a link pointing to the authors comment, (b) the reviewer grants 
the request by adding the exception without a comment,  or (c) the author adds 
the exception at the same time they make the request, and the reviewer 
implictly grants the request by not removing it. As the policy is written, (a) 
is already fine.

If based on usage feedback this is inconvenient and not providing value I’m 
open to adjusting it accordingly. Though did want to note that I’d be more 
interested in optimizing the workflow for `testing-exception-elsewhere` which 
also requires a comment (as it is expected / commonly used, like in Stacks) 
than `testing-exception-other` which should be used less often and require more 
scrutiny.

Brian

> Thanks,
> 
> JK
> 
> ___
> 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

A new testing policy for code landing in mozilla-central

[Brian Grinstead]

(This is a crosspost from firefox-dev)

Hi all,

We’re rolling out a change to the review process to put more focus on automated 
testing. This will affect you if you review code that lands in mozilla-central.

## TLDR

Reviewers will now need to add a testing Project Tag in Phabricator when 
Accepting a revision. This can be done with the “Add Action” → “Change Project 
Tags” UI in Phabricator. There's a screenshot of this UI at 
https://groups.google.com/g/firefox-dev/c/IlHNKOKknwU/m/zl6cOlT2AgAJ.

We’ve been piloting the policy for a few weeks with positive feedback from 
reviewers. Still, there may be some rough edges as we roll this out more 
widely. I’d like to hear about those, so please contact me directly or in the 
#testing-policy channel in Slack / Matrix if you have feedback or questions 
about how the policy should apply to a particular review.

We’re working on ways to make this more convenient in the UI and to prevent 
landing code without a tag in Lando. In the meantime if you'd like a reminder 
to add the tag while reviewing code, Nicolas Chevobbe has built a WebExtension 
that automatically opens up the Project Tags UI when appropriate at 
https://github.com/nchevobbe/phab-test-policy.

## Why?

Automated tests in Firefox and Gecko reduce risk and allow us to quickly and 
confidently improve our products.

While there’s a general understanding that changes should have tests, this 
hasn’t been tracked or enforced consistently. We think defining an explicit 
policy for including automated tests with code changes will help to achieve 
those goals.

Also, we’ll be able to better understand exactly why and when tests aren’t 
being added. This can help to highlight components that need new testing 
capabilities or technical refactoring, and features that require increased 
manual testing.

There are of course trade-offs to enforcing a testing policy. Tests take time 
to write, maintain, and run which can slow down day-to-day development. And 
given the complexity of a modern web browser, some components are impractical 
to test today (for example, components that interact with external hardware and 
software).

The policy below hopes to mitigate those by using a lightweight enforcement 
mechanism and the ability to exempt changes at the discretion of the code 
reviewer.

## Policy (This text is also located at 
https://firefox-source-docs.mozilla.org/testing/testing-policy/)

Everything that lands in mozilla-central includes automated tests by default. 
Every commit has tests that cover every major piece of functionality and 
expected input conditions.

One of the following Project Tags must be applied in Phabricator before 
landing, at the discretion of the reviewer:

* `testing-approved` if it has sufficient automated test coverage.
* One of `testing-exception-*` if not. After speaking with many teams across 
the project we’ve identified the most common exceptions, which are detailed 
below.

### Exceptions

* testing-exception-unchanged: Commits that don’t change behavior for end 
users. For example:
  * Refactors, mechanical changes, and deleting dead code as long as they 
aren’t meaningfully changing or removing any existing tests. Authors should 
consider checking for and adding missing test coverage in a separate commit 
before a refactor.
  * Code that doesn’t ship to users (for example: documentation, build scripts 
and manifest files, mach commands). Effort should be made to test these when 
regressions are likely to cause bustage or confusion for developers, but it’s 
left to the discretion of the reviewer.
* testing-exception-ui: Commits that change UI styling, images, or localized 
strings. While we have end-to-end automated tests that ensure the frontend 
isn’t totally broken, and screenshot-based tracking of changes over time, we 
currently rely only on manual testing and bug reports to surface style 
regressions.
* testing-exception-elsewhere: Commits where tests exist but are somewhere 
else. This requires a comment from the reviewer explaining where the tests are. 
For example:
  * In another commit in the Stack.
  * In a followup bug.
  * In an external repository for third party code.
  * When following the [Security Bug Approval 
Process](https://firefox-source-docs.mozilla.org/bug-mgmt/processes/security-approval.html)
 tests are usually landed later, but should be written and reviewed at the same 
time as the commit.
* testing-exception-other: Commits where none of the defined exceptions above 
apply but it should still be landed. This should be scrutinized by the reviewer 
before using it - consider whether an exception is actually required or if a 
test could be reasonably added before using it. This requires a comment from 
the reviewer explaining why it’s appropriate to land without tests. Some 
examples that have been identified include:
  * Interacting with external hardware or software and our code is missing 
abstractions to mock the interaction out.
  * Inability

Re: Intent to Prototype: Constructable Stylesheet Objects

[Brian Grinstead]

I’m happy to hear this - we have an interest in using this feature in the 
browser chrome.

We usually load chrome://global/skin/global.css as a link in the Shadow DOM for 
our chrome Custom Elements and rely on sync stylesheet loading from the 
(chrome-only) prototype cache, but would like to stop relying on this as it has 
led to bugs like Bug 1590280 (see 
https://bugzilla.mozilla.org/show_bug.cgi?id=1590280#c27 for more details).

Thanks,
Brian

> On Dec 5, 2019, at 2:03 PM, Erik Nordin  wrote:
> 
> Summary: Many web components
> use the functionality of Shadow DOM
> . Currently, In order for
> a stylesheet to take effect in a Shadow DOM, it must be specified using an
> HTML 

Can we change how [hidden] and [collapsed] work on XUL elements to more closely match HTML boolean attributes?

[Brian Grinstead]

In order to support moving more of our frontend to HTML, I'd like to propose 
that we change the way `hidden` and `collapsed` attributes and properties work 
on XUL elements and rewrite frontend consumers to adapt.

Currently, hidden and collapsed in XUL behave like:
* Only a value of true hides or collapses the element in the XUL markup. This 
is done in CSS rules 
(https://searchfox.org/mozilla-central/rev/1fe0cf575841dbf3b7e159e88ba03260cd1354c0/toolkit/content/minimal-xul.css#30-38)
 and there's also platform code that specifically looks for "true" on the 
attribute.
* Setting el.hidden = true or any truthy value will set hidden="true" in the 
markup
* Setting el.hidden = false or any falsy value will remove the hidden attribute

If we want to support more easily porting non Custom Element XUL elements (like 
) to HTML without requiring a Custom Element, I think we should switch 
the behavior to more closely match HTML:
* Setting any value to the hidden/collapsed HTML attribute will hide the 
element (we’d add CSS rules matching those in minimal-xul.css but in xul.css 
targeting html elements with the existence of the attribute instead of checking 
for “true”). 
* el.collapsed is not a thing: we’d rewrite consumers from `el.collapsed = val` 
to `el.toggleAttribute(“collapsed”, val)`
* Setting el.hidden = true or any truthy value will set hidden="" in the DOM
* Setting el.hidden = false or any falsy value will remove the hidden attribute
* The recommended way to write things in HTML markup is .  and  are also acceptable but  or any other value is _not_ acceptable as per 
https://html.spec.whatwg.org/multipage/common-microsyntaxes.html#boolean-attributes
 .
* The recommended way to write things in XHTML markup is 

Things I know we need to look more closely at before making a change:
* Check consumers that use XULStore to persist these values to make sure that 
they don't explicitly store [hidden=false] as a way to indicate that it's 
visible.

Are there reasons not to make this change, or things that need further 
investigation?

Note that this came out of a suggestion from ntim in 
https://phabricator.services.mozilla.com/D51168 - there's some more background 
in that patch.

Thanks,
Brian

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

Re: Can we remove xul:page?

[Brian Grinstead]

This has now landed. I've also filed
https://bugzilla.mozilla.org/show_bug.cgi?id=1584283 to do something
similar with xul:wizard.

Brian

On Mon, Sep 23, 2019 at 3:04 PM Brian Grinstead 
wrote:

> I've put up a patch showing what would change at
> https://bugzilla.mozilla.org/show_bug.cgi?id=1583377 /
> https://phabricator.services.mozilla.com/D46869.
>
> On Mon, Sep 23, 2019 at 2:53 PM Brian Grinstead 
> wrote:
>
>> We have 5 non-test consumers of  in m-c right now: 
>> https://searchfox.org/mozilla-central/search?q=%3Cpage&path=.xul.
>>
>> According to
>> https://developer.mozilla.org/en-US/docs/Archive/Mozilla/XUL/page, the
>> xul:page element is "similar to a window, except it should be used for XUL
>> files that are to be loaded into an iframe."
>>
>> But the only handling for page beyond being a generic XUL element I see
>> is:
>>
>> * One relevant match on nsGkAtoms::page
>> https://searchfox.org/mozilla-central/search?q=symbol:_ZN9nsGkAtoms4pageE&redirect=false
>> that lumps it in with other root xul elements at
>> https://searchfox.org/mozilla-central/rev/153feabebc2d13bb4c29ef8adf104ec1ebd246ae/layout/xul/nsBoxFrame.cpp#953
>> .
>> * Some CSS, but all of this applies to  and other roots as well:
>> https://searchfox.org/mozilla-central/search?q=%5Epage&case=false®exp=true&path=.css
>> .
>>
>> So, I'd like to find out if there's a reason we couldn't migrate the
>> consumers directly to  (with the ultimate goal of then migrating
>> those to ). Am I missing anything?
>>
>> Thanks,
>> Brian
>
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Can we remove xul:page?

[Brian Grinstead]

I've put up a patch showing what would change at
https://bugzilla.mozilla.org/show_bug.cgi?id=1583377 /
https://phabricator.services.mozilla.com/D46869.

On Mon, Sep 23, 2019 at 2:53 PM Brian Grinstead 
wrote:

> We have 5 non-test consumers of  in m-c right now: 
> https://searchfox.org/mozilla-central/search?q=%3Cpage&path=.xul.
>
> According to
> https://developer.mozilla.org/en-US/docs/Archive/Mozilla/XUL/page, the
> xul:page element is "similar to a window, except it should be used for XUL
> files that are to be loaded into an iframe."
>
> But the only handling for page beyond being a generic XUL element I see is:
>
> * One relevant match on nsGkAtoms::page
> https://searchfox.org/mozilla-central/search?q=symbol:_ZN9nsGkAtoms4pageE&redirect=false
> that lumps it in with other root xul elements at
> https://searchfox.org/mozilla-central/rev/153feabebc2d13bb4c29ef8adf104ec1ebd246ae/layout/xul/nsBoxFrame.cpp#953
> .
> * Some CSS, but all of this applies to  and other roots as well:
> https://searchfox.org/mozilla-central/search?q=%5Epage&case=false®exp=true&path=.css
> .
>
> So, I'd like to find out if there's a reason we couldn't migrate the
> consumers directly to  (with the ultimate goal of then migrating
> those to ). Am I missing anything?
>
> Thanks,
> Brian
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Can we remove xul:page?

[Brian Grinstead]

We have 5 non-test consumers of  in m-c right now: 
https://searchfox.org/mozilla-central/search?q=%3Cpage&path=.xul.

According to https://developer.mozilla.org/en-US/docs/Archive/Mozilla/XUL/page, 
the xul:page element is "similar to a window, except it should be used for XUL 
files that are to be loaded into an iframe."

But the only handling for page beyond being a generic XUL element I see is:

* One relevant match on nsGkAtoms::page 
https://searchfox.org/mozilla-central/search?q=symbol:_ZN9nsGkAtoms4pageE&redirect=false
 that lumps it in with other root xul elements at 
https://searchfox.org/mozilla-central/rev/153feabebc2d13bb4c29ef8adf104ec1ebd246ae/layout/xul/nsBoxFrame.cpp#953.
* Some CSS, but all of this applies to  and other roots as well: 
https://searchfox.org/mozilla-central/search?q=%5Epage&case=false®exp=true&path=.css.

So, I'd like to find out if there's a reason we couldn't migrate the consumers 
directly to  (with the ultimate goal of then migrating those to 
). Am I missing anything?

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

Re: Outline of a plan to remove all XUL documents from mozilla-central

[Brian Grinstead]

> On May 14, 2019, at 2:43 PM, Dave Townsend  wrote:
> 
> Which test files are we talking about here? If they are testing UI widgets, 
> and our long-term goal is to use html and not xhtml for the UI then those 
> tests should, at some point, be in html.

It's worth breaking the tests down into groups to consider separately. After 
going through this in more detail and based on discussion coming out of this 
post, it makes me think the variation where we do a blanket rename of all .xul 
files to .xhtml might be a better option - it'd certainly be less work at least.

Here are some of the groups of tests I see:

"Reftests" (Number: 364)

These are probably mostly safe to migrate, but there could be different CSS 
applying on elements (for instance if the root node is  and not 
). For that reason I think we could go straight to .xhtml (step 4) 
and then selectively apply the step 5 cleanups as long as they don't break 
tests.

"Crashtests" (Number: 147)

I think I'd go straight to .xhtml (step 4), and probably not even do any step 5 
cleanups. The reasoning is we don't want to go through each test one by one and 
decide if we are losing test coverage by making those changes.

“Mochitests that are specifically testing XML features” (Number: ???)

These would need to go straight to .xhtml (step 4) and then selectively apply 
the step 5 cleanups as long as they don't break tests.

"Mochitests that test built-in widgets" (Number: ~160)

I'm using tests in the `toolkit/content/tests/chrome` folder as a proxy for 
this. As you mention, we might want these to live in the same document the 
browser does (which will be xhtml, at least for now). Though in general 
something a widget that works in .xhtml should also work in .html - the 
breakage would mostly go the other way (i.e. if a widget relied on the 
prototype cache parser ignoring whitespace could work in xhtml but not in 
html). So I think you could argue either way - going straight to .xhtml would 
maintain the status quo.

"Mochitests that don't care about the test document" (Number: ???)

One example that I had envisioned going straight to .html are the 
mochitest-chrome tests in xpfe: 
https://bugzilla.mozilla.org/show_bug.cgi?id=1548152. I picked this folder as a 
starting point because there are only two tests there 
(https://searchfox.org/mozilla-central/source/xpfe/appshell/test/test_hiddenPrivateWindow.xul
 and 
https://searchfox.org/mozilla-central/source/xpfe/appshell/test/test_windowlessBrowser.xul),
 and as far as I can tell there's nothing in them that cares whether the test 
document itself is xul, xhtml, or html.

This is primarily the type of test I had in mind for (3).  My suspicion is that 
there are a lot of tests of that type across the tree. The trick will be 
finding them - Brendan has a script to find the most common DOM patterns across 
all the tests, and there are 32 other tests that match that exact DOM 
structure: 
https://gist.github.com/bgrins/22a448034748340042bdbf499e219a71#file-25-top-repeated-structures-txt-L426.
 It's not a guarantee that all of them would be safe to migrate, but 
spot-checking other tests there it seems to be the case. There are a bunch of 
other shared DOM structures that seem to be in a similar boat.

Brian

> On Tue, May 14, 2019 at 1:48 PM Brian Grinstead  
> wrote:
> There isn't any particular reason functionally to go to one vs the other but 
> I think we still generally prefer to get to plain .html if possible. The 
> reasoning is that it's more common and understood by engineers and tooling. 
> It also doesn't have XML-specific additions like CDATA in script tags.
> 
> That said, after talking through this again with Brendan we realized it may 
> not end up being worth the effort for existing test files. If we were able to 
> apply some of the incremental changes from step 5 to test .xhtml files that 
> may be good enough. If the cost was low to include step 3 (like, if we had a 
> tool that mostly automated away the process) then I’d prefer to do it. But it 
> could be pragmatic to skip step 3 (at least for test documents). We’ll be 
> working on some tooling anyway for step 5, so will spend a bit of time seeing 
> how hard it looks to be to automate the html doc conversion.
> 
> Brian
> 
> > On May 14, 2019, at 12:37 PM, Boris Zbarsky  wrote:
> > 
> > On 5/14/19 11:32 AM, Brian Grinstead wrote:
> >>>3. For files where there are no (important) XUL elements in the
> >>>markup, rename .xul->.html.
> > 
> > Brian,
> > 
> > Could you expand on why this is preferable (when possible) to renaming them 
> > to .xhtml?  Are there benefits to .html over .xhtml for our purposes here?  
> > Is this mostly around how we deal with our tests, as oppo

Re: Outline of a plan to remove all XUL documents from mozilla-central

[Brian Grinstead]

There isn't any particular reason functionally to go to one vs the other but I 
think we still generally prefer to get to plain .html if possible. The 
reasoning is that it's more common and understood by engineers and tooling. It 
also doesn't have XML-specific additions like CDATA in script tags.

That said, after talking through this again with Brendan we realized it may not 
end up being worth the effort for existing test files. If we were able to apply 
some of the incremental changes from step 5 to test .xhtml files that may be 
good enough. If the cost was low to include step 3 (like, if we had a tool that 
mostly automated away the process) then I’d prefer to do it. But it could be 
pragmatic to skip step 3 (at least for test documents). We’ll be working on 
some tooling anyway for step 5, so will spend a bit of time seeing how hard it 
looks to be to automate the html doc conversion.

Brian

> On May 14, 2019, at 12:37 PM, Boris Zbarsky  wrote:
> 
> On 5/14/19 11:32 AM, Brian Grinstead wrote:
>>>3. For files where there are no (important) XUL elements in the
>>>markup, rename .xul->.html.
> 
> Brian,
> 
> Could you expand on why this is preferable (when possible) to renaming them 
> to .xhtml?  Are there benefits to .html over .xhtml for our purposes here?  
> Is this mostly around how we deal with our tests, as opposed to actual parts 
> of the UI?
> 
> In general, the plan sounds great!
> 
> -Boris
> ___
> 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

Re: Outline of a plan to remove all XUL documents from mozilla-central

[Brian Grinstead]

It looks like the post lost formatting on the way - most importantly the
numbers next to each step and nesting in the list. I made a formatted
version of this post at
https://docs.google.com/document/d/1rEPcu7ei-kK5Dvt7sBgqa_vP37QFIUtVd33JkCK1mvM/edit
.

Here's a (hopefully) fixed version of the list:

1. Load all XUL documents as XHTML using the prototype cache. This doesn’t
require any file renaming, we will just detect a .xul file and act like
it’s .xhtml. This is tracked in bug 1550801. There are only a few known
changes required to make this work properly, for example:
a) Fixup calls to `document.createElement` to
`document.createXULElement` since the default namespace will now be HTML
(Bug 1551320).
b) Remove any remaining references to XULDocument. Most of these have
been removed already, but there are some callers that didn’t apply to the
main browser document that haven’t been removed yet (Bug 1551344)
c) This does _not_ require changing whitespace aware API callers (i.e.
calls to .childNodes->.children), since the prototype cache drops
whitespace in both XUL and XHMTL.
2. Delete the XULDocument implementation.
3. For files where there are no (important) XUL elements in the markup,
rename .xul->.html. This requires doing at least:
a) Fixup whitespace-aware API callers  (i.e. calls to
.childNodes->.children).
b) Write a tool to parse and automatically convert these documents as
much as possible. For instance, stop using self closing tags, migrate
``->``, convert ProcessingInstruction stylesheets to
``, etc.
c) Update bugs and comments referencing the old file name
4. For files that don’t match the heuristics from (3), rename .xul->.xhtml.
This is simpler than (3), since we don’t have to worry about whitespace
aware API, parsing issues, etc. In this case everything should work
identically with the file name changed. The main extra task here is:
d) Update bugs and comments referencing the old file name
5. For the now-xhtml documents from (4), apply a subset of changes from
(3). For instance, migrate ``->``, convert
ProcessingInstruction stylesheets to ``, etc).

On Tue, May 14, 2019 at 8:24 AM Brian Grinstead 
wrote:

> To prepare for browser.xhtml (bug 1533881
> <https://bugzilla.mozilla.org/show_bug.cgi?id=1533881>), we’ve been
> smoothing over differences between XUL documents and chrome HTML documents 
> (bug
> 1453783 <https://bugzilla.mozilla.org/show_bug.cgi?id=1453783>). We are
> now working out a plan for removing XUL documents entirely from
> mozilla-central (bug 1540278
> <https://bugzilla.mozilla.org/show_bug.cgi?id=1540278>). There are
> currently around 1530 xul documents in the tree
> <https://github.com/bgrins/xul-analysis/blob/master/file-lists/2019-05-09.txt>
> and around 1400 of these are in test directories.
>
> We’re early in the process right now but wanted to circulate the plan to
> let people know about upcoming changes and give a chance to provide
> feedback. If there aren’t objections, we think step 1 could be completed
> shortly after the merge (2019-05-20) and that we could start work on the
> following steps in short order. The outline of the plan is:
>
>
>1. Load all XUL documents as XHTML using the prototype cache. This
>doesn’t require any file renaming, we will just detect a .xul file and act
>like it’s .xhtml. This is tracked in bug 1550801
><https://bugzilla.mozilla.org/show_bug.cgi?id=1550801>. There are only
>a few known changes required to make this work properly, for example:
>1. Fixup calls to `document.createElement` to
>   `document.createXULElement` since the default namespace will now be 
> HTML (Bug
>   1551320 <https://bugzilla.mozilla.org/show_bug.cgi?id=1551320>).
>   2. Remove any remaining references to XULDocument. Most of these
>   have been removed already, but there are some callers that didn’t apply 
> to
>   the main browser document that haven’t been removed yet (Bug 1551344
>   <https://bugzilla.mozilla.org/show_bug.cgi?id=1551344>)
>   3. This does _not_ require changing whitespace aware API callers
>   (i.e. calls to .childNodes->.children), since the prototype cache drops
>   whitespace in both XUL and XHMTL.
>   2. Delete the XULDocument implementation.
>3. For files where there are no (important) XUL elements in the
>markup, rename .xul->.html. This requires doing at least:
>1. Fixup whitespace-aware API callers  (i.e. calls to
>   .childNodes->.children).
>   2. Write a tool to parse and automatically convert these documents
>   as much as possible. For instance, stop using self closing tags, migrate
>   ``->``, convert ProcessingInstruction stylesheets to
>   ``, etc.
>   3. Update bugs and comments

Outline of a plan to remove all XUL documents from mozilla-central

[Brian Grinstead]

To prepare for browser.xhtml (bug 1533881 
), we’ve been smoothing 
over differences between XUL documents and chrome HTML documents (bug 1453783 
). We are now working out 
a plan for removing XUL documents entirely from mozilla-central (bug 1540278 
). There are currently 
around 1530 xul documents in the tree 
 
and around 1400 of these are in test directories.

We’re early in the process right now but wanted to circulate the plan to let 
people know about upcoming changes and give a chance to provide feedback. If 
there aren’t objections, we think step 1 could be completed shortly after the 
merge (2019-05-20) and that we could start work on the following steps in short 
order. The outline of the plan is:

Load all XUL documents as XHTML using the prototype cache. This doesn’t require 
any file renaming, we will just detect a .xul file and act like it’s .xhtml. 
This is tracked in bug 1550801 
. There are only a few 
known changes required to make this work properly, for example:
Fixup calls to `document.createElement` to `document.createXULElement` since 
the default namespace will now be HTML (Bug 1551320 
). 
Remove any remaining references to XULDocument. Most of these have been removed 
already, but there are some callers that didn’t apply to the main browser 
document that haven’t been removed yet (Bug 1551344 
) 
This does _not_ require changing whitespace aware API callers (i.e. calls to 
.childNodes->.children), since the prototype cache drops whitespace in both XUL 
and XHMTL.
Delete the XULDocument implementation.
For files where there are no (important) XUL elements in the markup, rename 
.xul->.html. This requires doing at least:
Fixup whitespace-aware API callers  (i.e. calls to .childNodes->.children).
Write a tool to parse and automatically convert these documents as much as 
possible. For instance, stop using self closing tags, migrate 
``->``, convert ProcessingInstruction stylesheets to 
``, etc.
Update bugs and comments referencing the old file name
For files that don’t match the heuristics from (3), rename .xul->.xhtml. This 
is simpler than (3), since we don’t have to worry about whitespace aware API, 
parsing issues, etc. In this case everything should work identically with the 
file name changed. The main extra task here is:
Update bugs and comments referencing the old file name
For the now-xhtml documents from (4), apply a subset of changes from (3). For 
instance, migrate ``->``, convert ProcessingInstruction 
stylesheets to ``, etc).

A variation on this plan would be to move step 3 until after step 4 - 
essentially do a blanket rename of all .xul files to .xhtml shortly after step 
1 stuck. This would make it faster to complete the removal of xul files from 
the tree and get out of the intermediate state where we have .xul files loaded 
as HTML documents. However, this would come at the cost of more filename churn 
- tests that are ultimately destined for .html would be renamed 
.xul->.xhtml->.html instead of only .xul->.html. Renaming files has a cost 
because it can break in-flight patches, make it harder to search history, and 
requires updating bugs referencing the old file name (for example, for 
intermittent tracking bugs 
). Because of that, we 
are recommending the outline as-is instead of the variation above.

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

Re: Automatically including add_task into SimpleTest.js

[Brian Grinstead]

This has now landed, so feel free to start using add_task without any extra
boilerplate in mochitests.

AddTask.js is also now removed so if you had any in-flight patches that
were adding tests using it please remove the 

Automatically including add_task into SimpleTest.js

[Brian Grinstead]

This came up in 
https://groups.google.com/d/msg/mozilla.dev.platform/eptWENSn4wM/sjCWfj1LBwAJ:

> On Apr 1, 2019, at 12:38 PM, Brian Grinstead  wrote:
>> On Apr 1, 2019, at 12:15 PM, Boris Zbarsky  wrote:
>> I do have one other question on the templates: for mochitest-plain, add_task 
>> is a pretty rare thing to do, so I'm not sure defaulting the template to 
>> that makes sense.  For mochitest-chrome I'm not really sure; for 
>> mochitest-browse I agree that defaulting to add_task makes sense.
>> 
>> For the cases where we don't default to add_task (if any) we probably 
>> shouldn't include AddTask.js either, like we don't include other things that 
>> are helpful in only some test files (EventUtils.js, etc).
> 
> add_task does seem relatively rare on chrome right now, but I don’t think it 
> should be. It cleans up the boilerplate (no need to call 
> SimpleTest.waitForExplicitFinish and SimpleTest.finish, no need to figure out 
> how to hook up [onload], and gives async tests right off the bat). I’m not as 
> familiar with plain test, but I’d be fine to drop that from the template if 
> it’s not as useful.
> 
> I also think we should import the AddTask.js contents into SimpleTest.js to 
> make it available without an extra script, but that’s another discussion/bug 
> :).

I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1544051 to make this 
change. I think we should be including add_task anywhere that loads 
SimpleTest.js because:

• add_task is a nicer way to work with tests (it automatically calls 
SimpleTest.waitForExplicitFinish, waits for load, is async by default)
• the script doesn't have any side effects unless called
• it's available by default in other suites (mochitest-browser, 
xpcshell)

My proposal is to move the contents of AddTask.js into SimpleTest.js and then 
remove the separate file and any references to it. Would like to hear if there 
are reasons not to do this, otherwise I’ll make a patch to do so.

Brian

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

Re: Creating a new mach command for adding tests

[Brian Grinstead]

This has now landed (with initial support for xpcshell, mochitests, and web 
platform tests). Thanks to Andrew Halberstadt and James Graham for improving 
upon the initial prototype and making it easier to extend to new suites.

If the suite supports it (currently xpcshell and mochitests), we will also 
automatically write the new test into the manifest file. This means you can 
immediately run the test after running addtest - for example:

./mach addtest browser/components/extensions/test/xpcshell/test_xpcshell.js && 
./mach test test_xpcshell.js

The current global options are:
  --overwrite: recreate the file even if it already exists
  --editor: open the test in the specified editor (or default editor if it’s 
passed without an argument)
  --suite: explicitly specify the test suite (you generally shouldn’t need this 
since it will detect from the path)
  --doc: explicitly specify the document type (you generally shouldn’t need 
this since it will detect from the path)

If you’d like to add support for a new suite or find bugs / missing features, 
please block Bug 1540285.

Brian

> On Apr 1, 2019, at 11:36 AM, Brian Grinstead  wrote:
> 
> Based on my own experience and discussions with others, the workflow for 
> adding new mochitests isn't great. Commonly, it looks like: "copy/paste a 
> test in the same directory, add the new test to the relevant manifest file, 
> empty out the actual test bits, write your test". In my experience this is 
> prone to issues like forgetting to add the new test to the manifest, or not 
> fully replacing boilerplate like bug numbers from the copied test.
> 
> There's a script in tree I was unaware of until last week called 
> gen_template.pl that's intended to help here, but it does leave a few issues 
> open:
> 
> 1) It doesn't help with finding the manifest file and adding the new test to 
> it.
> 2) The boilerplate it generates is outdated (for example, it sets 
> type="application/javascript" even in HTML documents, it doesn't include 
> add_task, etc).
> 3) It supports only mochitest-chrome and mochitest-plain.
> 
> Last week I prototyped a new mach command to fix (1) and (2), and expand (3) 
> to include browser-chrome mochitests. If it's helpful, it could be extended 
> to more test types as well. When you run the command it will create a file 
> with the appropriate boilerplate and add it to the manifest file (chrome.ini, 
> mochitest.ini, browser.ini depending on the type). This way you can 
> immediately run the test with `./mach mochitest`. It looks like this:
> 
> ```
> # Chrome mochitests
> ./mach addtest js/xpconnect/tests/chrome/test_chrome.html
> ./mach addtest js/xpconnect/tests/chrome/test_chrome.xhtml
> ./mach addtest js/xpconnect/tests/chrome/test_chrome.xul
> 
> # Plain mochitests
> ./mach addtest js/xpconnect/tests/mochitest/test_plain.html
> ./mach addtest js/xpconnect/tests/mochitest/test_plain.xhtml
> ./mach addtest js/xpconnect/tests/mochitest/test_plain.xul
> 
> # Browser mochitests
> ./mach addtest browser/base/content/test/alerts/browser_mochitest.js
> ```
> 
> It's not landed because I’d like to get some feedback (see next paragraph), 
> but if you'd like to try it locally it can be applied from 
> https://phabricator.services.mozilla.com/D25482.
> 
> This change modifies the existing templates used by gen_template.pl and 
> removes the perl script. I think the template changes are mostly 
> uncontroversial, but there is one notable change that I'd like feedback on 
> before landing. We currently add a bug number to new tests generated with the 
> script (in 4 places). For example, see "{BUGNUMBER}" in 
> https://searchfox.org/mozilla-central/source/testing/mochitest/static/chrome.template.txt.
>  My patch removes this. The reasoning is:
> 
> 1) To tighten up the boilerplate and prevent the bug number from not being 
> updated in any or all of the spots when a test is created via copy/paste.
> 2) Require less information up front when generating the test (in case you 
> are building a test before filing a bug, or have a test not associated with 
> any one particular bug).
> 
> Links to bugs/comments/etc can be added in the test if they are relevant, but 
> I don't know that it's important enough to add another step in front of 
> getting a useful test case built. I did also consider adding a TODO comment 
> in the template to add a bug link (though in a single place instead of 4), 
> but not to require that information up front.
> 
> If I don’t hear objections on this point I’ll work towards getting it landed 
> soon.
> 
> Thanks,
> Brian
> 

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

Re: Proposal to remove unnecessary [type] attributes on script tags in mozilla-central

[Brian Grinstead]

> On Apr 8, 2019, at 8:55 PM, Cameron McCormack  wrote:
> 
> On Tue, Apr 9, 2019, at 1:39 PM, Brian Grinstead wrote:
>> I'd like to rewrite markup in the tree to avoid using the [type] 
>> attribute on 

Re: Proposal to remove unnecessary [type] attributes on script tags in mozilla-central

[Brian Grinstead]

Thank you. I will update the script(s) to use that list.

> On Apr 9, 2019, at 7:52 AM, Kartikaya Gupta  wrote:
> 
> On Mon, Apr 8, 2019 at 11:39 PM Brian Grinstead  
> wrote:
>> I've prepared a script that rewrites this across the tree. The script and 
>> the generated patch can be seen at 
>> https://bugzilla.mozilla.org/show_bug.cgi?id=1542877. The following 
>> directories are excluded:
>> 
>> - third_party
> 
> Just as a general note, there is a tools/rewriting/ThirdPartyPaths.txt
> that contains all the folders that should be skipped with these sorts
> of rewrites, because they are stuff vendored into m-c. In this case
> most of those folders probably don't have script tags, but still, it
> would be good to ensure you're not accidentally changing stuff in
> those folders.

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

Re: Proposal to remove unnecessary [type] attributes on script tags in mozilla-central

[Brian Grinstead]

> On Apr 9, 2019, at 2:31 AM, Anne van Kesteren  wrote:
> 
> On Tue, Apr 9, 2019 at 5:56 AM Cameron McCormack  wrote:
>> On Tue, Apr 9, 2019, at 1:39 PM, Brian Grinstead wrote:
>>> I'd like to rewrite markup in the tree to avoid using the [type]
>>> attribute on 

Re: Proposal to remove unnecessary [type] attributes on script tags in mozilla-central

[Brian Grinstead]

The reasoning is:
- If we don't rewrite existing test files then it will keep getting propagated 
in new tests as old ones get cloned.
- It's a small cleanup that makes existing tests slightly easier to read 
(especially when combined with other boilerplate reduction).
- I was under the impression until yesterday that type="application/javascript" 
was required on script tags in XUL documents (or maybe xul:script tags in any 
document). I assume I'm not the only one who's thought that or some variation 
of it - if we stop using the attribute then it would be one less out of the 
ordinary thing to deal with for Firefox development.

I'll think some more about if there’s a way to limit the risk of losing test 
coverage. Perhaps doing it in a couple of separate patches would result in a 
final patch that’s more manageable to review: first do simpletest.js and other 
common helper scripts tree-wide, then do full-replacement in folders that 
wouldn’t have a reason to test this specifically (browser/, devtools/, gfx/, 
particular dom/ subdirectories etc).

Brian

> On Apr 9, 2019, at 3:57 AM, Gijs Kruitbosch  wrote:
> 
> On 09/04/2019 11:16, James Graham wrote:
> 
>> I also wonder if there are other non-wpt tests that would be broken by such 
>> a change??
> 
> This was my thought as well. Is there a compelling reason to want to do this 
> rewrite for extant HTML mochitest test files?
> 
> ~ Gijs
> ___
> 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

Proposal to remove unnecessary [type] attributes on script tags in mozilla-central

[Brian Grinstead]

I'd like to rewrite markup in the tree to avoid using the [type] attribute on 

Re: Creating a new mach command for adding tests

[Brian Grinstead]

I don't think that having papercuts in the workflow for writing one type of 
test is the right way to nudge developers into writing another type. It also 
doesn't seem effective - otherwise people would be using the wpt-create tool to 
avoid jumping through hoops to add a mochitest.

That said, given there’s already a convention for this perhaps the tool as-is 
would be better named `./mach mochitest-create`. Based on Steve’s suggestion, 
if we did want a single API we could do something like:

# Attempt to automatically determine the type of test (mochitest-chrome, 
xpcshell, wpt, etc)
`./mach addtest path/to/test` 

# If you want to pass extra arguments specific to that type, then you use a 
subcommand:
`./mach addtest mochitest --flavor=chrome 
toolkit/components/windowcreator/test/test_chrome.html`
`./mach addtest wpt testing/web-platform/tests/accelerometer/test.html 
--long-timeout`

Brian

> On Apr 2, 2019, at 2:40 AM, James Graham  wrote:
> 
> On 01/04/2019 23:13, Steve Fink wrote:
>> On 4/1/19 11:36 AM, Brian Grinstead wrote:
>>> Based on my own experience and discussions with others, the workflow for 
>>> adding new mochitests isn't great. Commonly, it looks like: "copy/paste a 
>>> test in the same directory, add the new test to the relevant manifest file, 
>>> empty out the actual test bits, write your test". In my experience this is 
>>> prone to issues like forgetting to add the new test to the manifest, or not 
>>> fully replacing boilerplate like bug numbers from the copied test.
>>> 
>>> There's a script in tree I was unaware of until last week called 
>>> gen_template.pl that's intended to help here, but it does leave a few 
>>> issues open:
>>> 
>>> 1) It doesn't help with finding the manifest file and adding the new test 
>>> to it.
>>> 2) The boilerplate it generates is outdated (for example, it sets 
>>> type="application/javascript" even in HTML documents, it doesn't include 
>>> add_task, etc).
>>> 3) It supports only mochitest-chrome and mochitest-plain.
>>> 
>>> Last week I prototyped a new mach command to fix (1) and (2), and expand 
>>> (3) to include browser-chrome mochitests. If it's helpful, it could be 
>>> extended to more test types as well. When you run the command it will 
>>> create a file with the appropriate boilerplate and add it to the manifest 
>>> file (chrome.ini, mochitest.ini, browser.ini depending on the type). This 
>>> way you can immediately run the test with `./mach mochitest`.
>> It sounds great to me, but I'm wondering if the generic name is intentional 
>> or not. Various groups within Mozilla assume different things by 'test'. 
>> Is`mach addtest` intended to only be for mochitests? If so, then perhaps 
>> `mach addmochitest` is a better name, even it's a bit of mouthful. My 
>> reasoning is that there's already a distinction between `mach mochitest` and 
>> `mach test`, where the latter attempts to be general and support a bunch of 
>> different kinds of tests. Having `mach test` assume mochitests would be 
>> highly confusing to me, at least. (Though I'm not sure that `mach test` 
>> really works; it seems like I usually have to run the more specific command.)
> 
> I'm also pretty worried by this. For web-platform features ensuring interop 
> is critical and as such web-platform-tests should be preferred over 
> mochitests where possible. But every time we build features with a 
> mochitest-first approach it undermines that.
> 
> For web-platform-tests we already have ./mach wpt-create, so I think we 
> should either roll that functionality in to the new command as part of the 
> initial featureset or have one command per supported test type (i.e. call 
> this mach mochitest-create).
> ___
> 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

Re: Creating a new mach command for adding tests

[Brian Grinstead]

I was hoping that other types could be added over time, so making the name 
generic was intentional. It will already error if you try to add something that 
doesn’t look like a mochitest, but it’s a good suggestion to make that more 
obvious. I can file bugs for other test types and then include the bug # for in 
the error message to make that more likely to happen. I think this command is 
conceptually similar to `./mach test`, and we just won't have a direct analog 
to `./mach mochitest` - this would be done via something like `./mach addtest 
—type=browser` if you needed to explicitly set the type.

Brian

> On Apr 1, 2019, at 3:13 PM, Steve Fink  wrote:
> 
> On 4/1/19 11:36 AM, Brian Grinstead wrote:
>> Based on my own experience and discussions with others, the workflow for 
>> adding new mochitests isn't great. Commonly, it looks like: "copy/paste a 
>> test in the same directory, add the new test to the relevant manifest file, 
>> empty out the actual test bits, write your test". In my experience this is 
>> prone to issues like forgetting to add the new test to the manifest, or not 
>> fully replacing boilerplate like bug numbers from the copied test.
>> 
>> There's a script in tree I was unaware of until last week called 
>> gen_template.pl that's intended to help here, but it does leave a few issues 
>> open:
>> 
>> 1) It doesn't help with finding the manifest file and adding the new test to 
>> it.
>> 2) The boilerplate it generates is outdated (for example, it sets 
>> type="application/javascript" even in HTML documents, it doesn't include 
>> add_task, etc).
>> 3) It supports only mochitest-chrome and mochitest-plain.
>> 
>> Last week I prototyped a new mach command to fix (1) and (2), and expand (3) 
>> to include browser-chrome mochitests. If it's helpful, it could be extended 
>> to more test types as well. When you run the command it will create a file 
>> with the appropriate boilerplate and add it to the manifest file 
>> (chrome.ini, mochitest.ini, browser.ini depending on the type). This way you 
>> can immediately run the test with `./mach mochitest`.
> 
> It sounds great to me, but I'm wondering if the generic name is intentional 
> or not. Various groups within Mozilla assume different things by 'test'. 
> Is`mach addtest` intended to only be for mochitests? If so, then perhaps 
> `mach addmochitest` is a better name, even it's a bit of mouthful. My 
> reasoning is that there's already a distinction between `mach mochitest` and 
> `mach test`, where the latter attempts to be general and support a bunch of 
> different kinds of tests. Having `mach test` assume mochitests would be 
> highly confusing to me, at least. (Though I'm not sure that `mach test` 
> really works; it seems like I usually have to run the more specific command.)
> 
> Following the existing model exactly would mean adding `mach addmochitest`, 
> and additionally `mach addtest` that errors out if the path you give it leads 
> to a non-mochitest directory.
> 
> Or you could go for the even more generic `mach add mochitest` ... then you 
> could implement adding all kinds of things! ;-)
> 
> 
> ___
> 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

Re: Creating a new mach command for adding tests

[Brian Grinstead]

> On Apr 1, 2019, at 11:54 AM, Jared Hirsch <6...@mozilla.com> wrote:
> 
> This sounds great! I land features in the tree periodically, but infrequently 
> enough to forget lots of little details. It would really save time (mine and 
> the time of people I ping on IRC for help...) if our tooling simplified the 
> process of creating new tests.
> 
> I don't have any historical context on the reasons why the bug number was 
> added to the template, but removing it seems reasonable to me, fwiw.
> 
> One small feature request: could you add some basic usage documentation to 
> the firefox-source-docs as part of this patch? I'm sure I'll forget the right 
> sequence of commands in between uses ;-)

Glad to hear this would save time, makes me more sure I should push it forward.

For now I was planning to update the existing test template docs at 
https://developer.mozilla.org/en-US/docs/Mozilla/Projects/Mochitest#Test_templates
 to use the mach command, since that’s where a lot of related APIs are already 
documented. I’m open to the argument that this entire page should live on 
firefox-source-docs, but would like to decouple that from this command if 
possible :).

Brian

> Cheers,
> 
> Jared
> 
> 
> On Mon, Apr 1, 2019 at 11:36 AM Brian Grinstead  
> wrote:
> Based on my own experience and discussions with others, the workflow for 
> adding new mochitests isn't great. Commonly, it looks like: "copy/paste a 
> test in the same directory, add the new test to the relevant manifest file, 
> empty out the actual test bits, write your test". In my experience this is 
> prone to issues like forgetting to add the new test to the manifest, or not 
> fully replacing boilerplate like bug numbers from the copied test.
> 
> There's a script in tree I was unaware of until last week called 
> gen_template.pl that's intended to help here, but it does leave a few issues 
> open:
> 
> 1) It doesn't help with finding the manifest file and adding the new test to 
> it.
> 2) The boilerplate it generates is outdated (for example, it sets 
> type="application/javascript" even in HTML documents, it doesn't include 
> add_task, etc).
> 3) It supports only mochitest-chrome and mochitest-plain.
> 
> Last week I prototyped a new mach command to fix (1) and (2), and expand (3) 
> to include browser-chrome mochitests. If it's helpful, it could be extended 
> to more test types as well. When you run the command it will create a file 
> with the appropriate boilerplate and add it to the manifest file (chrome.ini, 
> mochitest.ini, browser.ini depending on the type). This way you can 
> immediately run the test with `./mach mochitest`. It looks like this:
> 
> ```
> # Chrome mochitests
> ./mach addtest js/xpconnect/tests/chrome/test_chrome.html
> ./mach addtest js/xpconnect/tests/chrome/test_chrome.xhtml
> ./mach addtest js/xpconnect/tests/chrome/test_chrome.xul
> 
> # Plain mochitests
> ./mach addtest js/xpconnect/tests/mochitest/test_plain.html
> ./mach addtest js/xpconnect/tests/mochitest/test_plain.xhtml
> ./mach addtest js/xpconnect/tests/mochitest/test_plain.xul
> 
> # Browser mochitests
> ./mach addtest browser/base/content/test/alerts/browser_mochitest.js
> ```
> 
> It's not landed because I’d like to get some feedback (see next paragraph), 
> but if you'd like to try it locally it can be applied from 
> https://phabricator.services.mozilla.com/D25482.
> 
> This change modifies the existing templates used by gen_template.pl and 
> removes the perl script. I think the template changes are mostly 
> uncontroversial, but there is one notable change that I'd like feedback on 
> before landing. We currently add a bug number to new tests generated with the 
> script (in 4 places). For example, see "{BUGNUMBER}" in 
> https://searchfox.org/mozilla-central/source/testing/mochitest/static/chrome.template.txt.
>  My patch removes this. The reasoning is:
> 
> 1) To tighten up the boilerplate and prevent the bug number from not being 
> updated in any or all of the spots when a test is created via copy/paste.
> 2) Require less information up front when generating the test (in case you 
> are building a test before filing a bug, or have a test not associated with 
> any one particular bug).
> 
> Links to bugs/comments/etc can be added in the test if they are relevant, but 
> I don't know that it's important enough to add another step in front of 
> getting a useful test case built. I did also consider adding a TODO comment 
> in the template to add a bug link (though in a single place instead of 4), 
> but not to require that information up front.
> 
> If I don’t hear objec

Re: Creating a new mach command for adding tests

[Brian Grinstead]

> On Apr 1, 2019, at 12:38 PM, Brian Grinstead  wrote:
>>> Links to bugs/comments/etc can be added in the test if they are relevant, 
>>> but I don't know that it's important enough to add another step in front of 
>>> getting a useful test case built. I did also consider adding a TODO comment 
>>> in the template to add a bug link (though in a single place instead of 4), 
>>> but not to require that information up front.
>> 
>> I think realistically getting to the bug through the version control history 
>> is reasonable, so there's not that much reason to have a bug link in the 
>> test itself.
>> 
>> I would further argue that the  in just about all our mochitests is 
>> pointless and could go from the template.
> 
> Would be happy to remove that - I was wondering why it was useful when I 
> started changing the templates and couldn’t think of a great reason.

I realize now that  is used to display a summary of the test from 
searchfox listing, so perhaps we shouldn't remove it. For example: 
https://searchfox.org/mozilla-central/rev/201450283cddc9e409cec707acb65ba6cf6037b1/accessible/tests/mochitest/test_descr.html#4
 sets the title as "nsIAccessible::description tests”, which can also be seen 
in the directory listing at 
https://searchfox.org/mozilla-central/source/accessible/tests/mochitest.

Brian

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

Re: Creating a new mach command for adding tests

[Brian Grinstead]

> On Apr 1, 2019, at 12:23 PM, Felipe G  wrote:
> 
> Something that would be a nice flare would be if it got the information from 
> the moz.build file to tell which component the new file would be related to. 
> 
> Definitely not essential, though.

I think that would be doable, though I don’t know off-hand the right python 
modules to use to get the information.

> And a question: will it also work if you're already in the right dir and just 
> specifies the filename? That's usually how I do it.
> All the examples were from the repo root dir so I thought to double check. 

I just checked and it does seem to work fine from a subfolder (assuming mach is 
on your path).

> Em seg, 1 de abr de 2019 15:55, Jared Hirsch <6...@mozilla.com> escreveu:
> This sounds great! I land features in the tree periodically, but
> infrequently enough to forget lots of little details. It would really save
> time (mine and the time of people I ping on IRC for help...) if our tooling
> simplified the process of creating new tests.
> 
> I don't have any historical context on the reasons why the bug number was
> added to the template, but removing it seems reasonable to me, fwiw.
> 
> One small feature request: could you add some basic usage documentation to
> the firefox-source-docs as part of this patch? I'm sure I'll forget the
> right sequence of commands in between uses ;-)
> 
> Cheers,
> 
> Jared
> 
> 
> On Mon, Apr 1, 2019 at 11:36 AM Brian Grinstead 
> wrote:
> 
> > Based on my own experience and discussions with others, the workflow for
> > adding new mochitests isn't great. Commonly, it looks like: "copy/paste a
> > test in the same directory, add the new test to the relevant manifest file,
> > empty out the actual test bits, write your test". In my experience this is
> > prone to issues like forgetting to add the new test to the manifest, or not
> > fully replacing boilerplate like bug numbers from the copied test.
> >
> > There's a script in tree I was unaware of until last week called
> > gen_template.pl that's intended to help here, but it does leave a few
> > issues open:
> >
> > 1) It doesn't help with finding the manifest file and adding the new test
> > to it.
> > 2) The boilerplate it generates is outdated (for example, it sets
> > type="application/javascript" even in HTML documents, it doesn't include
> > add_task, etc).
> > 3) It supports only mochitest-chrome and mochitest-plain.
> >
> > Last week I prototyped a new mach command to fix (1) and (2), and expand
> > (3) to include browser-chrome mochitests. If it's helpful, it could be
> > extended to more test types as well. When you run the command it will
> > create a file with the appropriate boilerplate and add it to the manifest
> > file (chrome.ini, mochitest.ini, browser.ini depending on the type). This
> > way you can immediately run the test with `./mach mochitest`. It looks like
> > this:
> >
> > ```
> > # Chrome mochitests
> > ./mach addtest js/xpconnect/tests/chrome/test_chrome.html
> > ./mach addtest js/xpconnect/tests/chrome/test_chrome.xhtml
> > ./mach addtest js/xpconnect/tests/chrome/test_chrome.xul
> >
> > # Plain mochitests
> > ./mach addtest js/xpconnect/tests/mochitest/test_plain.html
> > ./mach addtest js/xpconnect/tests/mochitest/test_plain.xhtml
> > ./mach addtest js/xpconnect/tests/mochitest/test_plain.xul
> >
> > # Browser mochitests
> > ./mach addtest browser/base/content/test/alerts/browser_mochitest.js
> > ```
> >
> > It's not landed because I’d like to get some feedback (see next
> > paragraph), but if you'd like to try it locally it can be applied from
> > https://phabricator.services.mozilla.com/D25482.
> >
> > This change modifies the existing templates used by gen_template.pl and
> > removes the perl script. I think the template changes are mostly
> > uncontroversial, but there is one notable change that I'd like feedback on
> > before landing. We currently add a bug number to new tests generated with
> > the script (in 4 places). For example, see "{BUGNUMBER}" in
> > https://searchfox.org/mozilla-central/source/testing/mochitest/static/chrome.template.txt.
> > My patch removes this. The reasoning is:
> >
> > 1) To tighten up the boilerplate and prevent the bug number from not being
> > updated in any or all of the spots when a test is created via copy/paste.
> > 2) Require less information up front when generating the test (in case you
> > are building a test before filing a bug, or hav

Re: Creating a new mach command for adding tests

[Brian Grinstead]

> On Apr 1, 2019, at 12:15 PM, Boris Zbarsky  wrote:
> 
> On 4/1/19 2:36 PM, Brian Grinstead wrote:
>> When you run the command it will create a file with the appropriate 
>> boilerplate and add it to the manifest file (chrome.ini, mochitest.ini, 
>> browser.ini depending on the type).
> 
> Brian, thank you for putting this together!
> 
> I have one concern with the mach bits themselves: It looks like the way the 
> type-detection works is that it looks for "browser_" in the test name, and if 
> that's present uses browser.ini.  Otherwise it uses chrome.ini if it's 
> present in the dir, else mochitest.ini if it's present, else errors out.
> 
> We have a fair number of dirs that have both a chrome.ini _and_ a 
> mochitest.ini, and defaulting to chrome.ini for those dirs seems odd. It 
> might be better to error out of the filename is test_foo and the dir has both 
> chrome.ini and mochitest.ini and tell the developer to pick one or the other 
> explicitly.

Good catch, just fixed this in the latest version on phab:

```
> ./mach addtest toolkit/components/windowcreator/test/test_chrome.html
Error: directory contains both a chrome.ini and mochitest.ini. Please specify 
either `chrome` or `plain` with the --type argument.
> ./mach addtest toolkit/components/windowcreator/test/test_chrome.html 
> --type="chrome"
Adding a test of type `chrome` and doc type `html`
Please make sure to add the new test to your commit. You can now run the test 
with:
./mach mochitest toolkit/components/windowcreator/test/test_chrome.html
```

>> Links to bugs/comments/etc can be added in the test if they are relevant, 
>> but I don't know that it's important enough to add another step in front of 
>> getting a useful test case built. I did also consider adding a TODO comment 
>> in the template to add a bug link (though in a single place instead of 4), 
>> but not to require that information up front.
> 
> I think realistically getting to the bug through the version control history 
> is reasonable, so there's not that much reason to have a bug link in the test 
> itself.
> 
> I would further argue that the  in just about all our mochitests is 
> pointless and could go from the template.

Would be happy to remove that - I was wondering why it was useful when I 
started changing the templates and couldn’t think of a great reason.

> I do have one other question on the templates: for mochitest-plain, add_task 
> is a pretty rare thing to do, so I'm not sure defaulting the template to that 
> makes sense.  For mochitest-chrome I'm not really sure; for mochitest-browse 
> I agree that defaulting to add_task makes sense.
> 
> For the cases where we don't default to add_task (if any) we probably 
> shouldn't include AddTask.js either, like we don't include other things that 
> are helpful in only some test files (EventUtils.js, etc).

add_task does seem relatively rare on chrome right now, but I don’t think it 
should be. It cleans up the boilerplate (no need to call 
SimpleTest.waitForExplicitFinish and SimpleTest.finish, no need to figure out 
how to hook up [onload], and gives async tests right off the bat). I’m not as 
familiar with plain test, but I’d be fine to drop that from the template if 
it’s not as useful.

I also think we should import the AddTask.js contents into SimpleTest.js to 
make it available without an extra script, but that’s another discussion/bug :).
 
Brian


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

Creating a new mach command for adding tests

[Brian Grinstead]

Based on my own experience and discussions with others, the workflow for adding 
new mochitests isn't great. Commonly, it looks like: "copy/paste a test in the 
same directory, add the new test to the relevant manifest file, empty out the 
actual test bits, write your test". In my experience this is prone to issues 
like forgetting to add the new test to the manifest, or not fully replacing 
boilerplate like bug numbers from the copied test.

There's a script in tree I was unaware of until last week called 
gen_template.pl that's intended to help here, but it does leave a few issues 
open:

1) It doesn't help with finding the manifest file and adding the new test to it.
2) The boilerplate it generates is outdated (for example, it sets 
type="application/javascript" even in HTML documents, it doesn't include 
add_task, etc).
3) It supports only mochitest-chrome and mochitest-plain.

Last week I prototyped a new mach command to fix (1) and (2), and expand (3) to 
include browser-chrome mochitests. If it's helpful, it could be extended to 
more test types as well. When you run the command it will create a file with 
the appropriate boilerplate and add it to the manifest file (chrome.ini, 
mochitest.ini, browser.ini depending on the type). This way you can immediately 
run the test with `./mach mochitest`. It looks like this:

```
# Chrome mochitests
./mach addtest js/xpconnect/tests/chrome/test_chrome.html
./mach addtest js/xpconnect/tests/chrome/test_chrome.xhtml
./mach addtest js/xpconnect/tests/chrome/test_chrome.xul

# Plain mochitests
./mach addtest js/xpconnect/tests/mochitest/test_plain.html
./mach addtest js/xpconnect/tests/mochitest/test_plain.xhtml
./mach addtest js/xpconnect/tests/mochitest/test_plain.xul

# Browser mochitests
./mach addtest browser/base/content/test/alerts/browser_mochitest.js
```

It's not landed because I’d like to get some feedback (see next paragraph), but 
if you'd like to try it locally it can be applied from 
https://phabricator.services.mozilla.com/D25482.

This change modifies the existing templates used by gen_template.pl and removes 
the perl script. I think the template changes are mostly uncontroversial, but 
there is one notable change that I'd like feedback on before landing. We 
currently add a bug number to new tests generated with the script (in 4 
places). For example, see "{BUGNUMBER}" in 
https://searchfox.org/mozilla-central/source/testing/mochitest/static/chrome.template.txt.
 My patch removes this. The reasoning is:

1) To tighten up the boilerplate and prevent the bug number from not being 
updated in any or all of the spots when a test is created via copy/paste.
2) Require less information up front when generating the test (in case you are 
building a test before filing a bug, or have a test not associated with any one 
particular bug).

Links to bugs/comments/etc can be added in the test if they are relevant, but I 
don't know that it's important enough to add another step in front of getting a 
useful test case built. I did also consider adding a TODO comment in the 
template to add a bug link (though in a single place instead of 4), but not to 
require that information up front.

If I don’t hear objections on this point I’ll work towards getting it landed 
soon.

Thanks,
Brian

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

Re: Proposal to adjust testing to run on PGO builds only and not test on OPT builds

[Brian Grinstead]

Would this apply to talos as well? I’ve wondered before if we should care at 
all about opt-only talos regressions for platforms where we ship PGO. IME quite 
a number of talos changes (both improvements and regressions) only show up on 
one or the other, so dropping one would simplify things.

Brian

> On Jan 3, 2019, at 8:17 AM, jmaher  wrote:
> 
> I would like to propose that we do not run tests on linux64-opt, 
> windows7-opt, and windows10-opt.
> 
> Why am I proposing this:
> 1) All test regressions that were found on trunk are mostly on debug, and in 
> fewer cases on PGO.  There are no unique regressions found in the last 6 
> months (all the data I looked at) that are exclusive to OPT builds.
> 2) On mozilla-beta, mozilla-release, and ESR, we only build/test PGO builds, 
> we do not run tests on plan OPT builds
> 3) This will reduce the jobs (about 16%) we run which in turn reduces, cpu 
> time, money spent, turnaround time, intermittents, complexity of the 
> taskgraph.
> 4) PGO builds are very similar to OPT builds, but we add flags to generate 
> profile data and small adjustments to build scripts behind MOZ_PGO flag 
> in-tree, then we launch the browser, collect data, and repack our binaries 
> for faster performance.
> 5) We ship PGO builds, not OPT builds
> 
> What are the risks associated with this?
> 1) try server build times will increase as we will be testing on PGO instead 
> of OPT
> 2) we could miss a regression that only shows up on OPT, but if we only ship 
> PGO and once we leave central we do not build OPT, this is a very low risk.
> 
> I would like to hear any concerns you might have on this or other areas which 
> I have overlooked.  Assuming there are no risks which block this, I would 
> like to have a decision by January 11th, and make the adjustments on January 
> 28th when Firefox 67 is on trunk.
> ___
> 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

Re: Proposal to adjust testing to run on PGO builds only and not test on OPT builds

[Brian Grinstead]

Artifact builds don’t work with PGO, do they? When I do `-p all` on an artifact 
try push I get busted PGO builds (for example: 
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7f8ead55ca97821c60ef38af4dec01b8bff0fdf3&selectedJob=219655864).
 What's needed to make it work? Requiring a full build for frontend-only 
changes would increase the turnaround time and resource savings in (3).

Brian

> On Jan 3, 2019, at 8:17 AM, jmaher  wrote:
> 
> I would like to propose that we do not run tests on linux64-opt, 
> windows7-opt, and windows10-opt.
> 
> Why am I proposing this:
> 1) All test regressions that were found on trunk are mostly on debug, and in 
> fewer cases on PGO.  There are no unique regressions found in the last 6 
> months (all the data I looked at) that are exclusive to OPT builds.
> 2) On mozilla-beta, mozilla-release, and ESR, we only build/test PGO builds, 
> we do not run tests on plan OPT builds
> 3) This will reduce the jobs (about 16%) we run which in turn reduces, cpu 
> time, money spent, turnaround time, intermittents, complexity of the 
> taskgraph.
> 4) PGO builds are very similar to OPT builds, but we add flags to generate 
> profile data and small adjustments to build scripts behind MOZ_PGO flag 
> in-tree, then we launch the browser, collect data, and repack our binaries 
> for faster performance.
> 5) We ship PGO builds, not OPT builds
> 
> What are the risks associated with this?
> 1) try server build times will increase as we will be testing on PGO instead 
> of OPT
> 2) we could miss a regression that only shows up on OPT, but if we only ship 
> PGO and once we leave central we do not build OPT, this is a very low risk.
> 
> I would like to hear any concerns you might have on this or other areas which 
> I have overlooked.  Assuming there are no risks which block this, I would 
> like to have a decision by January 11th, and make the adjustments on January 
> 28th when Firefox 67 is on trunk.
> ___
> 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

Re: Bypassing about:home/privacy page loads on new test profile builds via mach

[Brian Grinstead]

Thanks for the list, that looks like a nice set of defaults. I wanted to add a 
few related things:

- In addition to setting prefs via machrc you can use `--setpref`. For example 
`./mach run --setpref browser.newtabpage.enabled=false` which would then 
override the value in your machrc file. Both ways to change prefs were added in 
https://bugzilla.mozilla.org/show_bug.cgi?id=1172574 
 so they behave similarly.
- You can use `./mach run --temp-profile` which is very handy for running 
multiple profiles at once. That will make a temporary profile folder in your 
objdir and then inherit prefs from the machrc file and setpref arg.
- `./mach settings` will print out all the available sections of the machrc 
file.

Brian

> On Oct 28, 2018, at 1:29 PM, Cameron McCormack  wrote:
> 
> On Tue, Oct 23, 2018, at 9:53 AM, Kyle Machulis wrote:
>> Adding the following prefs turns off all new profile about:home loads
>> and just starts the browser with about:blank:
>> 
>> [runprefs]
>> browser.startup.blankWindow=true
>> browser.newtabpage.enabled=false
>> browser.startup.firstrunSkipsHomepage=true
>> browser.startup.homepage=about:blank
>> datareporting.policy.dataSubmissionPolicyBypassNotification=true
> 
> This is great, thanks for the tip Kyle!
> ___
> 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

Re: Intent to implement and ship: HTMLMarqueeElement

[Brian Grinstead]

The next step for removing XBL in marquee will be to put the content inside of 
a UA Widget Shadow Root and then (ideally) drop the JS-implemented animation in 
favor of CSS animation. I expect that should be enough to fix 
https://bugzilla.mozilla.org/show_bug.cgi?id=306344.

Brian

> On Oct 14, 2018, at 5:29 PM, Karl Dubost  wrote:
> 
> 
> 
> Le 13 oct. 2018 à 02:56, Brian Grinstead  a écrit :
>> Summary: […] I intend to implement and ship HTMLMarqueeElement.
> 
> Very cool. And a support on that, from a webcompat standpoint of view, 
> because it seems a lot of Indian websites rely on it. The current 
> implementation has "performance" issues compared to other browsers where the 
> animation is a lot smoother.
> 
>> Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1425874
> 
> See https://bugzilla.mozilla.org/show_bug.cgi?id=306344
> 
> 
> -- 
> Karl Dubost, mozilla 💡 Webcompat
> http://www.la-grange.net/karl/moz
> 
> 
> 
> 
> 

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

Intent to implement and ship: HTMLMarqueeElement

[Brian Grinstead]

Summary: Motion is a key component of modern web design, and  is the 
premier... just kidding. Gecko currently ships  as an HTMLDivElement 
with the web-exposed properties attached via in-content XBL [0][1]. As part of 
the process of removing in-content XBL, I intend to implement and ship 
HTMLMarqueeElement.

This will allow us to expose properties on the element through WebIDL instead 
of through ``. This will lead to 
behavior changes with the current implementation 
(https://bugzilla.mozilla.org/show_bug.cgi?id=1425874#c10), although those 
changes will generally bring Firefox into compliance with the spec and with 
other implementations - many currently failing web platform tests should start 
passing [2][3].

The event handlers (onbounce, onfinish, onstart) will still be implemented in 
XBL for the time being - the actual XBL removal will happen in subsequent bugs.

There's an open question about how to treat the `loop` property. As outlined at 
https://bugzilla.mozilla.org/show_bug.cgi?id=1425874#c14, the spec defines a 
no-op when setting `loop` to a number that is <= 0 and not -1, which is similar 
to what our current implementation does. In this case Chrome, Safari, and Edge 
throw, so we may want to adapt this behavior.

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

Link to standard: 
https://html.spec.whatwg.org/multipage/obsolete.html#the-marquee-element

Platform coverage: All

Target Release: 65

Preference behind which this will be implemented: None

Is this feature enabled by default in sandboxed iframes? Yes

DevTools bug: None

Do other browser engines implement this?
- Chrome: since version 1
- Edge: yes
- IE: since version 2
- Safari: since version 1.2

web-platform-tests: 
https://github.com/web-platform-tests/wpt/tree/master/html/obsolete/requirements-for-implementations/the-marquee-element-0

Is this feature restricted to secure contexts? No

Brian

[0]: https://searchfox.org/mozilla-central/source/dom/html/HTMLDivElement.cpp
[1]: https://searchfox.org/mozilla-central/search?q=xbl-marquee.xml&path=
[2]: 
https://searchfox.org/mozilla-central/rev/28fe656de6a022d1fc01bbd3811023fca99cf223/testing/web-platform/meta/html/dom/interfaces.https.html.ini#628-746
[3]: 
https://searchfox.org/mozilla-central/rev/1ce4e8a5601da8e744ca6eda69e782318afab54d/testing/web-platform/meta/html/dom/reflection-obsolete.html.ini#8-2829

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

Re: Browser Architecture Newsletter #7 (S02E02)

[Brian Grinstead]

> On Sep 20, 2018, at 9:10 AM, Kris Maglione  wrote:
> 
> On Thu, Sep 20, 2018 at 09:02:09AM -0700, Nicholas Alexander wrote:
>> On Thu, Sep 20, 2018 at 7:25 AM smaug  wrote:
>>> > I'm reminded of https://bugzilla.mozilla.org/show_bug.cgi?id=618912 but
>>> > IIRC there were similar experiments back then on desktop, and basic html
>>> > chrome was significantly faster than basic xul chrome.
>>> That bug seems to be more about the layout.
>>> 
>>> 
>>> https://screenshotscdn.firefoxusercontent.com/images/d1753829-3ebd-4c42-a757-14757051accf.png
>>> is
>>> the latest numbers I've seen. That isn't pgo, so may or many not be very
>>> accurate, but the regression is
>>> very significant.
>>> 
>> 
>> I'm not expert in these areas, so I hope the experts chime in, but I think
>> there are lots of trade offs here.  I believe that you are correct: the XUL
>> prototype cache and similar mechanisms significantly impact browser startup
>> and related metrics.  But there is a general belief, which I do not have
>> references for, that the HTML widget set is either faster than or will be
>> faster than the XUL widget set.  Certainly folks believe that effort should
>> be put into optimizing core Web Platform technologies (rather than
>> Mozilla-specific extensions).
> 
> We can't afford a startup or window opening performance regression. If 
> switching to HTML gives us other performance improvements, that's great, but 
> it can't come at the cost of performance in other areas that we've worked so 
> hard to get into reasonable shape.

To be clear: we’re not going to ship a new browser window with performance 
regressions. Right now we are working on fixing broken tests in browser.xhtml 
so that we can do an apples-to-apples performance comparison with browser.xul. 
Once we do that we’ll start fixing performance regressions (if any). This work 
is tracked in Bug 1453783.

I did some talos pushes to get an idea of the magnitude of regressions to 
expect when disabling individual parts of the prototype cache while using 
browser.xul:
- nsXULPrototypeCache::PutXBLDocumentInfo and 
nsXULPrototypeCache::PutStyleSheet are the most impactful parts of the cache, 
and they continue to work even in chrome HTML documents AFAICT. Disabling them 
causes 5-20% regressions on a bunch of tests (https://mzl.la/2pq3Sh5 and 
https://mzl.la/2NWXJ9L).
- nsXULPrototypeCache::PutScript seems to have no impact on talos 
(https://mzl.la/2xp1BXK).
- Disabling nsXULPrototypeCache::PutPrototype causes the ~3% tpaint and 
ts_paint_heavy regressions that smaug is pointing to (https://mzl.la/2xq1JpI). 
That's disabling the prototype cache while still using a XUL document, so not 
the same environment we’d be in with an HTML document.

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

Re: XUL Overlays Removed

[Brian Grinstead]

> On Jul 13, 2018, at 3:05 PM, Frank-Rainer Grahl  wrote:
> 
> > This is just one piece of the broader XUL removal effort, but it does
> > highlight that things can be simpler in a post-XUL world.
> 
> Well I agree that cleaning up overlay usage was overdue. Otherwise the simple 
> post XUL world world is just dumb. Removing things without a functional 
> replacement and putting in spaghetthi code seems to be the current mantra. 
> Preprocessing with include files is even worse.

The preprocessor was a perfectly good replacement for the way we used overlays 
used in the Firefox UI - it makes it easier to look at a document and figure 
out which resources are going to be loaded into it. Work to simplify our chrome 
documents (including reducing preprocessing) can now be done from a foundation 
where we know which documents will be affected by changes.

Brian

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

Attempting to define XUL removal

[Brian Grinstead]

Removing XUL has been talked about for a long time, but I think it means 
different things to different people. Since I’ve spent some time working on 
XUL-related projects I’m going to summarize what it means to me as a way to 
share what’s currently planned and to get feedback.

XUL is a collection of technologies, so there isn’t going to be a single 
project that “removes XUL”. What we’d like to do is align browser chrome 
development more closely with web development, and to simplify Gecko by 
focusing on the web platform. Each of the projects below should get us a step 
closer to that, and each has its own priority and timeline.

One of the successes of XUL is that some of the good parts are now part of the 
web platform. For those features, “removal” looks like a migration to the 
standard. Once the old feature isn’t used anymore in chrome, we intend to 
remove the platform feature (and may remove parts of it along the way as those 
parts become unused).

XBL bindings can be replaced with ‘webbier' alternatives like Web Components or 
plain JS. It’s a large project, and we are actively working on removing 
bindings from the frontend and supporting Web Components in the platform 
(metabug , tracking 
dashboard ).
XUL flexbox can now be emulated as CSS flexbox. You can try `./mach run 
--setpref layout.css.emulate-moz-box-with-flex=true` to see how it looks. The 
main work to do now is to improve performance on CSS flexbox to get to parity 
on the TART test. That perf work is mostly unassigned - if you’d like to help 
with this project please get in touch or take a look at the perf metabug 
(emulation metabug , perf 
metabug ).

Other features will be removed and not replaced:

XUL Overlays are no longer necessary for supporting XUL addons and we can 
simplify a bunch of things by moving away from them. This is actively being 
worked on, and we expect to be able to remove platform support in a matter of 
weeks (metabug ).
XUL Templates were removed at the beginning of 2018. There was only one 
consumer on the frontend which got replaced with simpler JS, and the platform 
support (around 40K LOC) was removed as well (bug 
).

There is another set of features in XUL that aren’t part of the web platform 
but also provide functionality that can’t be removed without a replacement that 
works in chrome HTML. For example:

Localization via external DTD files. There are a bunch of reasons why we are 
transitioning the localization infrastructure to use Project Fluent, including 
that DTDs are tied to the XML parser and can’t be used from HTML. This work is 
actively happening, currently focused on strings in about:preferences but 
planned to expand across the browser chrome in coming quarters (metabug 
, fluent homepage 
).
Generating native menus and popups. This is in the planning phase.
Supporting special tags like  and  (and many more). This is in 
the planning phase.
Extra features that make the application tick, like XULStore and fastload. This 
is in the planning phase.

Much of that work is still being planned, so there aren’t a lot of specifics to 
report. Features that are blocking the ability to load a top-level HTML window, 
or that are blocking other work like XBL removal will get prioritized above 
others.

Thanks,
Brian

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

Re: Proposal to replace the NS_ASSERT function in JS with console.assert

[Brian Grinstead]

This has now landed in Bug 1431050 - remaining consumers were either
migrated to console.assert or changed to throw an exception.

You can continue to use console.assert instead of NS_ASSERT in JS. If you
find a case where console logging from chrome code doesn't do what you
expect please let me know.

Thanks,
Brian

On Mon, Feb 26, 2018 at 10:24 AM, Marco Bonardo 
wrote:

> On Mon, Feb 26, 2018 at 7:02 PM, Brian Grinstead 
> wrote:
> > console.assert doesn't throw an exception, and neither does NS_ASSERT.
> So I don’t think replacing consumers with exceptions is correct if we want
> to keep the current behavior. But I guess the intent for places code in bug
> 1431050 is to change the behavior and make those assertions throw?
>
> By looking at the patch, some consumers used it wrongly expecting it
> to interrupt the code (that likely threw a few lines later). While
> other code threw an exception explicitly after NS_ASSERT.
> Thus the common need seems to be "print a stack somewhere and throw".
> For that we can just wrap console.assert into a local util that
> throws, it's not a problem.
> Afaik, the only other advantage of NS_ASSERT was to make the assertion
> very visible to Nightly users, so they could report a bug. But that
> also had downsides.
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Proposal to replace the NS_ASSERT function in JS with console.assert

[Brian Grinstead]

console.assert doesn't throw an exception, and neither does NS_ASSERT. So I 
don’t think replacing consumers with exceptions is correct if we want to keep 
the current behavior. But I guess the intent for places code in bug 1431050 is 
to change the behavior and make those assertions throw? If that’s the case then 
there’s no need to migrate to console.assert at all and we can instead delete 
the module in that bug.

Brian

> On Feb 26, 2018, at 9:45 AM, Marco Bonardo  wrote:
> 
> We were already working with a contributor to remove NS_ASSERT and debug.js in
> https://bugzilla.mozilla.org/show_bug.cgi?id=1431050 and we were just
> replacing those few calls with simple exceptions.
> The usage is not particularly spread in the codebase and the benefits
> in ruining Nightly testers life with continuous modal dialogs
> (actually happened in the past) were not great.
> 
> console.assert may be a good replacement, we can leverage the existing
> bug to use that instead of simple exceptions. The bug is currently
> proceeding slower than hoped.
> Does console.assert also throw an exception?
> 
> Cheers,
> Marco
> 
> 
> On Mon, Feb 26, 2018 at 6:37 PM, Brian Grinstead  
> wrote:
>> There have been some improvements to the console API for chrome callers 
>> recently:
>> 
>> * The WebIDL implementation is available as a global in JSMs - no need to 
>> import Console.jsm (bug 1425574)
>> * Chrome console API calls print to stdout if 
>> "browser.dom.window.dump.enabled" is true (bug 1439686)
>> * `console.assert` prints the callstack (bug 1439677)
>> 
>> After these changes, I don't see a benefit to using `NS_ASSERT` instead of 
>> `console.assert`. So I'd like to propose that we either:
>> 
>> 1) Delete the debug.js module [0] and rewrite the ~100 callers of its 
>> `NS_ASSERT` function [1] to use `console.assert` instead.
>> 2) Make the existing `NS_ASSERT` function an alias for `console.assert`.
>> 
>> I’d like hear any feedback or objections before doing either, but my 
>> preference is (1) so that we can unify chrome JS on using the WebIDL console 
>> APIs. This is basically what we plan on doing with Console.jsm (bug 1430810).
>> 
>> The main behavior difference I see is that in `NS_ASSERT` we early return 
>> based on the update channel [2] before dumping to stdout, but after doing 
>> `Cu.reportError`. So this means if the assertion runs in a release build 
>> then it won't attempt to dump the message + stack. But since 
>> "browser.dom.window.dump.enabled" will be false by default that case, 
>> `console.assert` should have the same behavior. The difference would be if 
>> you were running a release build with "browser.dom.window.dump.enabled" 
>> flipped - in this case with `NS_ASSERT` you would not get a message in 
>> stdout but with `console.assert` you would.
>> 
>> Thanks,
>> Brian
>> 
>> [0]: https://searchfox.org/mozilla-central/source/toolkit/modules/debug.js
>> [1]: 
>> https://searchfox.org/mozilla-central/search?q=NS_ASSERT&case=true&path=js*
>> [2]: 
>> https://searchfox.org/mozilla-central/rev/ecb86060b4c5a9808798b81a57e79e821bb47082/toolkit/modules/debug.js#41
>> 
>> ___
>> 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

Proposal to replace the NS_ASSERT function in JS with console.assert

[Brian Grinstead]

There have been some improvements to the console API for chrome callers 
recently:

* The WebIDL implementation is available as a global in JSMs - no need to 
import Console.jsm (bug 1425574)
* Chrome console API calls print to stdout if "browser.dom.window.dump.enabled" 
is true (bug 1439686)
* `console.assert` prints the callstack (bug 1439677)

After these changes, I don't see a benefit to using `NS_ASSERT` instead of 
`console.assert`. So I'd like to propose that we either:

1) Delete the debug.js module [0] and rewrite the ~100 callers of its 
`NS_ASSERT` function [1] to use `console.assert` instead.
2) Make the existing `NS_ASSERT` function an alias for `console.assert`.

I’d like hear any feedback or objections before doing either, but my preference 
is (1) so that we can unify chrome JS on using the WebIDL console APIs. This is 
basically what we plan on doing with Console.jsm (bug 1430810).

The main behavior difference I see is that in `NS_ASSERT` we early return based 
on the update channel [2] before dumping to stdout, but after doing 
`Cu.reportError`. So this means if the assertion runs in a release build then 
it won't attempt to dump the message + stack. But since 
"browser.dom.window.dump.enabled" will be false by default that case, 
`console.assert` should have the same behavior. The difference would be if you 
were running a release build with "browser.dom.window.dump.enabled" flipped - 
in this case with `NS_ASSERT` you would not get a message in stdout but with 
`console.assert` you would.

Thanks,
Brian

[0]: https://searchfox.org/mozilla-central/source/toolkit/modules/debug.js
[1]: https://searchfox.org/mozilla-central/search?q=NS_ASSERT&case=true&path=js*
[2]: 
https://searchfox.org/mozilla-central/rev/ecb86060b4c5a9808798b81a57e79e821bb47082/toolkit/modules/debug.js#41

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

Re: Proposal to remove the xpfe autocomplete bindings and the xpfe/components/autocomplete directory

[Brian Grinstead]

This has now landed - it was moved into comm-central in Bug 1418512 and removed 
from mozilla-central in Bug 1417119.

Brian

> On Nov 14, 2017, at 9:25 AM, Brian Grinstead  wrote:
> 
> That makes sense. I went ahead and filed Bug 1417119 for the removal from 
> m-c, and we can coordinate a move to comm-central there.
> 
> Brian
> 
>> On Nov 14, 2017, at 1:04 AM, Frank-Rainer Grahl  wrote:
>> 
>> They are used in comm-central suite and owned by suite. We are currently 
>> evaluating moving them to comm-central. There is also corresponding css in 
>> the tree.
>> 
>> Regards
>> Frank-Rainer Grahl
>> 
>> 
>> 
>> 
>> Brian Grinstead wrote:
>>> While tracking down the XBL bindings in the tree, I noticed that there are 
>>> 4 inside the xpfe/ directory at 
>>> https://github.com/mozilla/gecko-dev/blob/master/xpfe/components/autocomplete/resources/content/autocomplete.xml.
>>> I'd like to remove those bindings, and AFAICT we aren't using them in 
>>> Firefox. Are there any objections to removing the code from m-c?
>>> Thanks,
>>> Brian
>> ___
>> 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

Re: Proposal to remove the xpfe autocomplete bindings and the xpfe/components/autocomplete directory

[Brian Grinstead]

That makes sense. I went ahead and filed Bug 1417119 for the removal from m-c, 
and we can coordinate a move to comm-central there.

Brian

> On Nov 14, 2017, at 1:04 AM, Frank-Rainer Grahl  wrote:
> 
> They are used in comm-central suite and owned by suite. We are currently 
> evaluating moving them to comm-central. There is also corresponding css in 
> the tree.
> 
> Regards
> Frank-Rainer Grahl
> 
> 
> 
> 
> Brian Grinstead wrote:
>> While tracking down the XBL bindings in the tree, I noticed that there are 4 
>> inside the xpfe/ directory at 
>> https://github.com/mozilla/gecko-dev/blob/master/xpfe/components/autocomplete/resources/content/autocomplete.xml.
>> I'd like to remove those bindings, and AFAICT we aren't using them in 
>> Firefox. Are there any objections to removing the code from m-c?
>> Thanks,
>> Brian
> ___
> 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

Proposal to remove the xpfe autocomplete bindings and the xpfe/components/autocomplete directory

[Brian Grinstead]

While tracking down the XBL bindings in the tree, I noticed that there are 4 
inside the xpfe/ directory at 
https://github.com/mozilla/gecko-dev/blob/master/xpfe/components/autocomplete/resources/content/autocomplete.xml.

I'd like to remove those bindings, and AFAICT we aren't using them in Firefox. 
Are there any objections to removing the code from m-c?

Thanks,
Brian

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

Re: De-XBL Plans

[Brian Grinstead]

> On Oct 22, 2017, at 3:35 AM, smaug  wrote:
> 
> On 10/21/2017 11:45 PM, Yura Zenevich wrote:
>> I would also like to bring to the team's attention another force worth
>> being on the radar (in terms of "forces on the system") - accessibility.
>> One theme that seems to consistently happen with re-writes such as the ones
>> from xul to React is regressions in terms of accessibility of newly
>> re-written components.
> 

Thanks, I’ve submitted a PR to the plan to add this point.

> Yeah, this is important. I would imagine custom elements in XUL (which might 
> then internally use shadow DOM too) would implement the same
> a11y interfaces what XBL implements now.

I agree - if we migrate the logic inside of bindings (i.e. formatAccessKey on 
) and continue to use XUL Elements this should limit the risk of 
regressions. Two places we should take a closer look at:

1) Ensure that anonymous XBL content is not somehow handled differently from an 
accessibility standpoint compared to normal DOM children in a Custom Element 
(or content inside Shadow DOM)
2) We may need to emulate the [role] attribute on a binding definition, which 
does map to some accessibility code. Do we need to change the way we create the 
appropriate Accessible class for Custom Elements? It does look at 
XBLBindingRole as a way to choose which role to assign (i.e. role=”xul:toolbar” 
on the binding definition). For Custom Elements, it may be easier to use tag 
names to determine the role, or somehow store it in the custom element registry 
to avoid setting a ‘role’ attribute on each node during the connectedCallback.
** 
https://dxr.mozilla.org/mozilla-central/rev/69e24c678a28dc46a4c1bda3ff04b2f6106ff71a/toolkit/content/widgets/button.xml#12
** 
https://dxr.mozilla.org/mozilla-central/rev/69e24c678a28dc46a4c1bda3ff04b2f6106ff71a/accessible/base/nsAccessibilityService.cpp#1380

>> 
>> thanks,
>> yura
>> 
>> On Sat, Oct 21, 2017 at 6:14 AM, Philipp Kewisch  wrote:
>> 
>>> On 10/20/17 7:47 PM, Dave Townsend wrote:
 For some time now we've been talking about moving away from XUL and XBL.
 The browser architecture team has been hard at work figuring out how to
>>> go
 about doing that and we're ready to share the first of our proposals more
 widely. We have developed a plan to remove XBL from Firefox. It's been
 through a successful design review with some of the key engineers and now
 is the time for more comments if you have them. We're planning to start
>>> some
 of the work this quarter with it really ramping up next quarter.
 
 Take a look at the plan
 >> text/0007-xbl-design-review-packet.html>
 and let us know what you think. There are a couple of areas where we are
 still investigating concerns:
>>> 
>>> I very much welcome this plan, especially the fact that Web Components
>>> is part of the replacement. Last time I asked, it sounded like Web
>>> Components was still on the way of being reimplemented and pending some
>>> spec work. In following the webcomponents bug I see there has been
>>> constant progress.
>>> 
>>> Nevertheless, I'd appreciate if someone could comment on how far along
>>> the Web Components implementation is. Is it now following the agreed
>>> upon version of the spec (I suspect yes), and is the implementation
>>> stable enough that you would consider it ready to ship? What is the next
>>> big milestone for Web Components?
>>> 
>>> Thunderbird/Lightning uses a lot of XBL components as well that I would
>>> love to get rid of, I am looking forward to making things more
>>> compatible with the future.
>>> 
>>> Thanks,
>>> Philipp
>>> 
>>> 
>>> 
>>> ___
>>> 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

Re: Using the accesskey attribute with HTML elements in browser chrome

[Brian Grinstead]

> On Jun 19, 2017, at 6:58 PM, Ehsan Akhgari  wrote:
> 
> On 06/19/2017 04:28 PM, Brian Grinstead wrote:
>> I was wondering what would need to be done in order to use the accesskey 
>> attribute on HTML elements in the browser chrome. Here are some of the 
>> differences between the attribute in HTML and XUL that I've found so far:
>> 
>> 1) In XUL 
>> (https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/Attribute/accesskey)
>>  the shortcut is `Control + key` on Windows and `Command + key` on Mac. In 
>> HTML 
>> (https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/accesskey)
>>  the shortcut is `Alt + Shift + key` on Windows and `Control + Alt + key` on 
>> Mac. Would we want to change the HTML shortcut (in browser chrome) to match 
>> XUL behavior?
> As far as I can tell this was last changed/discussed in 
> https://bugzilla.mozilla.org/show_bug.cgi?id=340902, and Mats may be the 
> right person to comment on it (but do note that this hasn't changed since 
> Firefox 2.)
> 
>> 2) The FAQ & Policies page 
>> (https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/XUL_Accesskey_FAQ_and_Policies)
>>  conflicts the with previous point when it says that accesskey is only 
>> available on Mac in HTML. If this is correct, would we want to disable 
>> accesskey functionality on Mac for HTML (in browser chrome) to maintain the 
>> current behavior, or would it working be a benefit of using HTML?
> That documentation seems incorrect.  Mac 
> <https://searchfox.org/mozilla-central/rev/7cc377ce3f0a569c5c9b362d589705cd4fecc0ac/modules/libpref/init/all.js#4040>
>  uses different defaults than other OSes 
> <https://searchfox.org/mozilla-central/rev/7cc377ce3f0a569c5c9b362d589705cd4fecc0ac/modules/libpref/init/all.js#2518>.
>  Not sure why.  :-)
>> 3) The formatting / underlining is controlled by the label-control xbl 
>> binding at 
>> http://searchfox.org/mozilla-central/source/toolkit/content/widgets/text.xml#66.
>>  Presumably this is something that would need to be reimplemented for HTML 
>> elements in order to use them in the browser chrome.
> The underlining?  Could that not be achieved using CSS?

It’s a little too complicated for CSS alone. The code in xbl splits up the 
element so that it can underline the first instance of the character only.  And 
if it can’t find the character than it appends at the end of the label in 
parenthesis while doing some extra special casing if the label ends in a colon.

>> Am I missing anything else?
> on Windows, the OS native widgets have a custom way of handling the 
> underlining IIRC to draw it only when Alt is pressed or something along those 
> lines and at some point we also had a bug about it to implement it for XUL, 
> but I'm not sure if we ever did that and I can't find the bug right now, but 
> something to double check perhaps.
> 

Thanks for the info, I didn’t realize that. I did some looking around and found 
https://bugzilla.mozilla.org/show_bug.cgi?id=25894 which I think is what you 
are referring to. Although apparently that system setting is off by default as 
of Windows 7.

> Cheers,
> Ehsan

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

Using the accesskey attribute with HTML elements in browser chrome

[Brian Grinstead]

I was wondering what would need to be done in order to use the accesskey 
attribute on HTML elements in the browser chrome. Here are some of the 
differences between the attribute in HTML and XUL that I've found so far:

1) In XUL 
(https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/Attribute/accesskey) 
the shortcut is `Control + key` on Windows and `Command + key` on Mac. In HTML 
(https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/accesskey) 
the shortcut is `Alt + Shift + key` on Windows and `Control + Alt + key` on 
Mac. Would we want to change the HTML shortcut (in browser chrome) to match XUL 
behavior?

2) The FAQ & Policies page 
(https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/XUL_Accesskey_FAQ_and_Policies)
 conflicts the with previous point when it says that accesskey is only 
available on Mac in HTML. If this is correct, would we want to disable 
accesskey functionality on Mac for HTML (in browser chrome) to maintain the 
current behavior, or would it working be a benefit of using HTML?

3) The formatting / underlining is controlled by the label-control xbl binding 
at 
http://searchfox.org/mozilla-central/source/toolkit/content/widgets/text.xml#66.
 Presumably this is something that would need to be reimplemented for HTML 
elements in order to use them in the browser chrome.

Am I missing anything else?

Thanks,
Brian


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

Re: Deprecating XUL in new UI

[Brian Grinstead]

A few things we had to handle when going through this process for the devtools 
UI:
1) Already mentioned to but for panels we added a new module that falls back to 
XUL panels for now
2) For context menu handling we we added a new module similar to the Menu API 
from Electron that falls back to  and  elements.  We didn't 
need to worry about top-level menus or overlays though.
3) Not impossible but needed a replacement for  /  elements for 
registering keyboard shortcuts.
4) Not impossible but agree with Jarda about mixing XUL and HTML flex - many of 
the bugs blocking the de-XUL meta 
(https://bugzilla.mozilla.org/show_bug.cgi?id=1263741) were layout issues 
created when an individual UI component was converted, and finding workarounds 
can be time consuming and require trial and error.

For 1 and 2, in our case it was worth changing the frontend to use new modules 
even though they are ultimately still relying on XUL.  That let us provide an 
HTML shim when running the panel in a content tab, and hopefully will let us 
change the implementation if/when there are ways to do it without XUL.

Brian

> On Jan 16, 2017, at 12:43 PM, Dave Townsend  wrote:
> 
> One of the things I've been investigating since moving back to the desktop 
> team is how we can remove XUL from the application as much as possible. The 
> benefits for doing this are varied, some obvious examples:
> 
> * XUL is a proprietary standard and we barely maintain it.
> * Shallower learning curve for new contributors who may already know and use 
> HTML.
> * HTML rendering is more optimized in the platform than XUL.
> * Further integration of Servo code may require dropping XUL altogether.
> 
> The necessary first step of reducing XUL use is to stop adding any more UI 
> that uses XUL and here I'm talking about wholly new UI, additions to 
> browser.xul and other existing UI are a separate concern. What do folks think 
> about making this a rule for new projects in the near future?
> 
> Of course there are some features missing from HTML that make this impossible 
> in some cases right now. Some that I've already found:
> 
> * HTML has no support for popup panels like XUL does. The devtools team have 
> been working around this but they are still dependent on XUL right now.
> * iframe elements don't have the same capabilities that the XUL browser 
> element does and we use that in some UI.
> * Top level menus on OSX are currently only able to be defined with XUL 
> elements. This only affects UI that uses top-level windows and most of our 
> new UI is in-content so may be less of an issue.
> 
> What other features do we depend on in XUL that I haven't listed?
> ___
> firefox-dev mailing list
> firefox-...@mozilla.org
> https://mail.mozilla.org/listinfo/firefox-dev

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

Re: Deprecating XUL in new UI

[Brian Grinstead]

You can still use dtd files in XHTML as long as it’s chrome-privileged.  A lot 
of the about pages are doing this (aboutNetError.xhtml and others: 
https://dxr.mozilla.org/mozilla-central/search?q=path%3Axhtml+dtd).

Brian

> On Jan 17, 2017, at 3:34 AM, zbranie...@mozilla.com wrote:
> 
> One more thing that XUL gives us is L10n.
> 
> With HTML, we can use .properties to load localization resources and inject 
> them into HTML, but I believe this to be a very inelegant solution with a 
> surprisingly high risk of bugs.
> 
> We do have an l10n framework called L20n that is supposed to replace DTD and 
> works in raw XUL and HTML binding elements to l10n messages with 
> `data-l10n-id` attribute.
> 
> Our plan was to target post-quantum release to refactor the XUL code to 
> switch from DTD to L20n, but we could also just introduce the new approach 
> and use it for new code already, while waiting for post-quantum to transition 
> the old code.
> 
> zb.
> ___
> 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

Re: Intent to remove: Error Console

[Brian Grinstead]

Filed bug 1278368 to remove the code and bug 1278372 to remove the component 
from bugzilla.

Brian

> On Jun 8, 2016, at 7:57 AM, Benjamin Smedberg  wrote:
> 
> \o/
> 
> Is there a bug to track this code removal?
> 
> --BDS
> 
> On Mon, Jun 6, 2016 at 4:04 PM, Brian Grinstead  
> wrote:
> There is an Error Console feature in toolkit that's been replaced by the 
> Browser Console.  We'd like to remove associated code in 
> toolkit/components/console/ and the component in bugzilla (Toolkit: Error 
> Console).  This will also remove the —jsconsole command line flag for 
> consumers that don’t use devtools.
> 
> The code isn't used at all in Firefox, as discussed in 
> https://groups.google.com/forum/#!topic/mozilla.dev.developer-tools/XYPqQ58ndX4/discussion.
>   It’s also now possible to migrate usages to the Browser Console i.e. 
> Seamonkey is no longer using it as of bug 1223341.
> 
> Brian
> ___
> 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

Re: Intent to remove: Error Console

[Brian Grinstead]

> On Jun 6, 2016, at 1:20 PM, Emma Humphries  wrote:
> 
> There's 77 open bugs in the component, http://mzl.la/1YbBKto, how would you 
> like to handle those? 

The majority of these look like frontend-related bugs and feature requests 
which could be closed once the code is removed.  I suggest a mass-closure with 
a comment to re-categorize it into Developer Tools: Console if they are also 
applicable to the Browser Console.

> -- Emma
> 
> On Mon, Jun 6, 2016 at 1:04 PM, Brian Grinstead  
> wrote:
> There is an Error Console feature in toolkit that's been replaced by the 
> Browser Console.  We'd like to remove associated code in 
> toolkit/components/console/ and the component in bugzilla (Toolkit: Error 
> Console).  This will also remove the —jsconsole command line flag for 
> consumers that don’t use devtools.
> 
> The code isn't used at all in Firefox, as discussed in 
> https://groups.google.com/forum/#!topic/mozilla.dev.developer-tools/XYPqQ58ndX4/discussion.
>   It’s also now possible to migrate usages to the Browser Console i.e. 
> Seamonkey is no longer using it as of bug 1223341.
> 
> Brian
> ___
> 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

Intent to remove: Error Console

[Brian Grinstead]

There is an Error Console feature in toolkit that's been replaced by the 
Browser Console.  We'd like to remove associated code in 
toolkit/components/console/ and the component in bugzilla (Toolkit: Error 
Console).  This will also remove the —jsconsole command line flag for consumers 
that don’t use devtools.

The code isn't used at all in Firefox, as discussed in 
https://groups.google.com/forum/#!topic/mozilla.dev.developer-tools/XYPqQ58ndX4/discussion.
  It’s also now possible to migrate usages to the Browser Console i.e. 
Seamonkey is no longer using it as of bug 1223341.

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

Re: help:browser tool box debugger is not stopped

[Brian Grinstead]

(moving over to the developer tools list)

Thanks for reporting this - I’ve heard about a couple of issues similar to this 
lately, but haven’t been able to reproduce it myself so another data point is 
great.  Do you see the problem both with and without e10s enabled?  Also, does 
it happen on a clean profile?

Brian

> On Apr 25, 2016, at 6:59 AM, oonuma ryouyu  wrote:
> 
> hello!
> 
> I am trying to debugging on browserToolBox.
> 
> I open browserToolBox and open debugger and setting breakpoint, but don't 
> stop.
> because set statement was passed.
> 
> Then I restart firefox, but it was not memorize breakpoint.
> 
> Thanks for help.
> ___
> 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

Re: PSA: Cancel your old Try pushes

[Brian Grinstead]

> On Apr 15, 2016, at 12:44 PM, Jim Blandy  wrote:
> 
> On Fri, Apr 15, 2016 at 12:36 PM, Jonas Sicking  wrote:
> 
>> We could also make the default behavior be to cancel old pushes. And
>> then enable push message syntax for opting in to not cancelling.
>> 
>> 
> This could be very frustrating (and cause farm work to be wasted) if it
> happened accidentally.
> 
> Perhaps it would be less error-prone to require an explicit choice of
> overlapping or cancellation, and immediately reject pushes that haven't
> chosen one or the other, for bugs that already have running try pushes.

Explicit choice sounds good.  I'd rather it not be required before pushing if 
it were a prompt.  If it were a try syntax option I would likely set "do not 
cancel" as a default to prevent accidental cancellation.

My proposal: enhance mach try to surface this information and allow convenient 
cancellation.  And if it were pushed using some other manner like a web ui or 
hg push then the default behavior would remain as it is today (to prevent 
losing work by default).  So something like this:

   $ ./mach try args

   remote: adding changesets
   remote: adding manifests
   remote: adding file changes
   remote: added 2 changesets with 1 changes to 2 files (+1 heads)
   remote: recorded push in pushlog
   remote:
   remote: View your changes here:
   remote:   https://hg.mozilla.org/try/rev/REV1
   remote:   https://hg.mozilla.org/try/rev/REV2
   remote:
   remote: Follow the progress of your build on Treeherder:
   remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=REV2
   remote: recorded changegroup in replication log in 0.093s

   Please help make Try faster by canceling old jobs.  You have two existing 
builds for this bug:

 https://treeherder.mozilla.org/#/jobs?repo=try&revision=REV3 (50% complete)
 https://treeherder.mozilla.org/#/jobs?repo=try&revision=REV4 (90% complete)

   Would you like to cancel these jobs? (Y/N)

Brian

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

Re: Dominator tree memory analysis now in Nightly

[Brian Grinstead]

> On Jan 14, 2016, at 3:01 PM, Nick Fitzgerald  wrote:
> Also, what is the plan for making tools like the Memory view appear in a 
> default install? In other words, without having to go to the Settings. I 
> worry that users may not know to click on the Settings and just think that 
> other browsers offer more tools than we do.
> 
> ​This is a good conversation to have, but I don't have any good answers. We 
> have a lot of panels in the devtools, many of which are fairly niche, and it 
> isn't clear how to simultaneously give them visibility and not overwhelm 
> users.
> 
> Console, debugger, performance: these panels are ​a clear "always show" to 
> me. The web audio tool is very niche and so I don't think it would make sense 
> to show it by default. The memory tool falls in between in my mind. I'm open 
> to suggestions!

There are some designs in flight that would make selecting a non-default panel 
much easier by adding an arrow after the last one, which would open a popup 
with all panels available.  So you could avoid a trip to the settings panel.  
I’ve filed a bug 1239859 for this since I couldn’t find another one on file.

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

Re: Dynamic Logging

[Brian Grinstead]

> On Jan 8, 2016, at 5:50 PM, Mike Hommey  wrote:
> 
> On Fri, Jan 08, 2016 at 05:32:48PM -0800, Eric Rahm wrote:
>> Hi Folks-
>> 
>> With bug 1233881  we
>> landed the ability turn on logging via prefs.
>> 
>> Lets say you have a log module "Foo", if you add a "logging.Foo" pref and
>> set it to "Debug" you will now see all output from the Foo log module that
>> is of Debug and higher importance.
>> 
>> Why is this so cool? Well now you don't need to restart your browser to
>> enable logging [1]. You also don't have to set env vars to enable logging
>> [2].
>> 
>> There is one caveat: if you don't use LazyLogModule and friends, you don't
>> get dynamic logging. So go update your loggers!
> 
> I now wish we had the same for the three or so different logging systems
> in use in javascript.

By the way, with Console.jsm you can do something similar:

  let {ConsoleAPI} = Cu.import("resource://gre/modules/Console.jsm", {});
  let console = new ConsoleAPI({
maxLogLevelPref: "myPref.loglevel",
  });
  console.log(“Message”);

The UITour code does this, and sets it’s log level pref to “error” by default 
so only console.error messages will be shown.  Then you can flip that pref to 
“all” or “log” at runtime to enable logging for that module. 

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

Re: Intent to ship: Service Workers with FetchEvent

[Brian Grinstead]

Also, webconsole logging for Service Workers is preffed behind 
devtools.webconsole.filter.serviceworkers, which will be enabled by default in 
https://bugzilla.mozilla.org/show_bug.cgi?id=1201962.

Brian

> On Nov 20, 2015, at 10:34 AM, Ben Kelly  wrote:
> 
> In Firefox 44 we intend to enable Service Workers and FetchEvents by
> default on desktop and android.  These features will not be enabled on
> Firefox OS yet.
> 
> They has been developed behind the following preferences:
> 
>  dom.serviceWorkers.enabled
>  dom.serviceWorkers.interception.enabled
>  dom.serviceWorkers.interception.opaque.enabled
> 
> Chrome has been shipping Service Workers with FetchEvent since Chrome 40.
> 
> Unfortunately, I can't find a previous intent to implement for service
> workers.
> 
> Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1226686
> Standard:
> https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html
> 
> Please let me know if you have any questions are concerns.
> 
> Thanks.
> 
> Ben
> ___
> 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

Re: Intent to remove: mochitest- mach commands

[Brian Grinstead]

Does this include subsuites like devtools?  There is currently a bug on file in 
which `mach mochitest` doesn’t work with devtools tests (1122590).  Until that 
is resolved, if mochitest-devtools was removed then running these tests would 
become the harder-to-remember `./mach mochitest browser/devtools --subsuite 
devtools` as opposed to `./mach mochitest-devtools browser/devtools`.  I'd 
request that mochitest-devtools remains in place until it's possible to run 
simply `mach mochitest browser/devtools`

Thanks,
Brian

> On May 13, 2015, at 12:54 PM, Andrew Halberstadt  
> wrote:
> 
> As mentioned previously in another post, work is under way to remove the
> flavor specific mochitest commands (e.g mach mochitest-plain, mach
> mochitest-browser etc.). This is being tracked in bug 1164597 [1].
> Bug description pasted below:
> 
>> As part of an effort to make running tests as easy and intuitive as
>> possible, I'd like to consolidate all of the mochitest- mach
>> commands into the single |mach mochitest| command.
>> 
>> Currently |mach mochitest| will automatically detect the flavor of
>> mochitests you wish to run. If you specify a directory that contains
>> multiple flavors, each flavor will be run in sequence. For example:
>> 
>> ./mach mochitest dom/indexedDB
>> 
>> runs all the mochitest-chrome, mochitest-browser-chrome and
>> mochitest-plain tests in that directory. The flavor can be limited
>> by passing in -f (--flavor). So to only run the chrome tests in that
>> directory you'd use:
>> 
>> ./mach mochitest -f chrome dom/indexedDB
>> 
>> Commands of the form |mach mochitest-| will be removed:
>> 
>> ./mach mochitest-plain -> ./mach mochitest -f plain ./mach
>> mochitest-browser -> ./mach mochitest -f browser etc..
>> 
>> I believe this will make running tests easier and more intuitive.
>> The fact that different flavors of mochitest exist will become a
>> background implementation detail. Instead, an emphasis will be
>> placed on running directories of tests, or tests related to certain
>> features. The knowledge of which test belongs to which flavor will
>> no longer be required, but of course the ability to run specific
>> flavors will still be there for those who need it.
>> 
>> The described behaviour for |mach mochitest| above, already exists
>> today. This bug is about making it the defacto way to run mochitests
>> (maybe aside from |mach test|). This bug will also add b2g and
>> android support to |mach mochitest|.
>> 
>> Exceptions to this will be robocop, webapprt-chrome and
>> webapprt-content which for technical reasons will not be rolled into
>> |mach mochitest| (for now).
> 
> 
> Please let me know if you have any questions or concerns,
> -Andrew
> 
> [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1164597
> ___
> 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

Re: what is new in talos, what is coming up

[Brian Grinstead]

The upcoming changes sound great!  Is there currently a way (or plans to add a 
way) to track regressions / improvements for a single measurement within a 
test?  I see that in perfherder I can add these measurements to a graph 
(http://mzl.la/1E17Zyo ) but it’s hard to distinguish 
between normal variation across runs and an actual regression by looking at the 
graph.

Brian

> On May 1, 2015, at 9:40 AM, jma...@mozilla.com wrote:
> 
> It is always hard to advertise every change, but there are enough changes to 
> post a brief summary.
> 
> What has changed:
> 1) :bgrins has added a new talos test 'damp' - devtools at maximum 
> performance!  This was done in 
> https://bugzilla.mozilla.org/show_bug.cgi?id=1150215
> 2) Talos doesn't run on OSX 10.6 anymore thanks to 
> https://bugzilla.mozilla.org/show_bug.cgi?id=990490
> 3) Talos runs on OSX 10.10 on main branches and is slowly replacing OSX 10.8 
> out with uplifts on older branches.
> 4) A few android tests were turned off, now we run 3 of them- ones that are 
> useful for tracking performance
> 
> What changes are coming up:
> 1) We are planning on reducing the pages we test for android tests, this is 
> mostly because the raw data is nearly identical to other pages.  For 
> reference, this would reduce tp4m in half and tsvgx by a few pages.  This 
> work will help us reduce over load as we transition from panda boards to real 
> devices this summer.
> 2) compare-talos is being integrated into perfherder- This will be detailed 
> in a blog post with examples of how to use it for investigating regressions 
> and fixes.  This work has been taking place in 
> https://bugzilla.mozilla.org/show_bug.cgi?id=1142680#c50
> 3) This quarter we will stop reporting to datazilla, this leaves graph server 
> and perfherder (the performance view inside of treeherder).
> 4) talos counters will be reviewed and cleaned up this quarter as part of 
> https://bugzilla.mozilla.org/show_bug.cgi?id=1156907
> 5) we will start having compare-talos style views in bugs we file for 
> performance regressions 
> (https://bugzilla.mozilla.org/show_bug.cgi?id=1150616).
> 
> All feedback is welcome.  With active work on new tools and deliverables to 
> make things better, you can expect more changes.  Of course, the more ideas 
> and feedback we get, the better !!
> ___
> 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