[desktop] Bugs logged by Desktop Release QA in the last 7 days

2019-04-01 Thread Mihai Boldan

Hello,

Sorry for the spam. Forgot to mention the bugs from github.

Here's the list of new issues found and filed by the Desktop Release QA 
team in the last 7 days.
Additional details on the team's priorities last week, as well as the 
plans for the current week are available at: https://tinyurl.com/y4n4rovw.

Bugs logged by Desktop Release QA in the last 7 days:

Firefox: Sync
NEW - https://bugzil.la/1539784 - Clicking the “Cancel” button from the 
Master Password Prompt while trying to disconnect sync account leads to 
a “Failed to sanitize” error


Firefox: Preferences
NEW - https://bugzil.la/1540085 - Manage Cookies and Site Data values 
are offset


Firefox: Security
NEW - https://bugzil.la/1540164 - Cert error page is displayed after 
restart when toggling the specific prefs


Core: DOM: Events
NEW - https://bugzil.la/1538651 - Selected strings in Office's365 Word 
documents do not get replaced if inputting new text instead


Core: DOM: Events
NEW - https://bugzil.la/1538652 - Selecting strings in Office's365 Word 
documents in any way shifts the focus out of the document


Core: Layout: Positioned
NEW - https://bugzil.la/1539089 - Text on Blacklist button from 
about:url-classifier is not centered


Core: Audio/Video: Playback
NEW - https://bugzil.la/1539098 - [Youtube] The sound is briefly 
interrupted if the user enters full screen mode


Core: Audio/Video: Playback
NEW - https://bugzil.la/1539182 - IMDB video freezes if another IMDB 
video is paused in a different tab


Core: Layout
NEW - https://bugzil.la/1539401 - [Netflix] Dragging control buttons 
from thumbnail videos makes them appear near the cursor outside the window


Core: Audio/Video
NEW - https://bugzil.la/1539419 - Glitches when enlarging videos on Facebook

Core: Graphics
NEW - https://bugzil.la/1539431 - [Linux] "Emoticon picker" from outlook 
is cut off


Core: Panning and Zooming
NEW - https://bugzil.la/1539443 - [Reddit] Page elements flash on fast 
scrolling


DevTools: Inspector: Rules
NEW - https://bugzil.la/1538646 - Pasting copied selector+declarations 
when creating a new rule should create the rule *and* add the declarations


DevTools: Inspector: Changes
ASSIGNED - https://bugzil.la/1538648 - Select all option selects even 
the [Copy All Changes] button


DevTools: Inspector: Changes
NEW - https://bugzil.la/1538683 - Export changes - improper focus on 
Copy All Changes button


DevTools: Inspector: Changes
NEW - https://bugzil.la/1539127 - [Track Changes] Properties with 
[--name] selector format populate the whole list in the changes tab


Toolkit: Crash Reporting
NEW - https://bugzil.la/1539772 - [Win] Info section in crash reporter 
is selected after crash-restart


Toolkit: Add-ons Manager
NEW - https://bugzil.la/1540087 - Pressing D button while inside any 
addon options will disabled it


External Software Affecting Firefox: Other
NEW - https://bugzil.la/1538706 - Suggested McAfee Antivirus product 
variant to be used for testing


External Software Affecting Firefox: Other
NEW - https://bugzil.la/1540133 - [Win] - "Profile missing" error spam 
after Kaspsersky old install -> Update to Internet Security version


WebExtensions: General
NEW - https://bugzil.la/1540106 - [Evernote Web Clipper] Toggling 
between Allow/Don't Allow buttons erases content for additional add-on 
specific settings


OPEN - #231  - 
"Facebook Container" string is duplicated in the overflow menu


OPEN - #234 - 
Facebook Container's panel displays horizontal and vertical scrollbars 
if opened via the Overflow Menu


This is available as a Bugzilla bug list as well: 
https://tinyurl.com/yxv58p48.


Regards,
Mihai Boldan



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


Re: Intent to implement and ship: CSS 'prefers-color-scheme' media feature

2019-04-01 Thread Gijs Kruitbosch
Are there also plans to revisit our longstanding 
foreground/background/link/visited-link colouring prefs (as well as 
their companion use_system_colors pref) in light of this set of changes? 
They've never really worked very well, nobody has really taken on fixing 
their UX, and it would be nice if we could switch to something that was 
better supported.


~ Gijs

On 15/02/2019 20:38, Mats Palmgren wrote:

Summary:
The 'prefers-color-scheme' media feature is way for pages to detect
if the user prefers a light or dark color theme.  The values are
'light', 'dark', and 'no-preference'.  If the "resist fingerprinting"
feature is active we always match 'light'.  If the media type is
'print' we always match 'light'.  Otherwise, we try to determine
a value from the user's current "desktop theme".  This should work
well on recent versions of OSX and Windows.  On Linux, we don't
know how to determine a value from the system so we match
'no-preference' there - help wanted:
https://bugzilla.mozilla.org/show_bug.cgi?id=1525775

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

Link to standard:
https://drafts.csswg.org/mediaqueries-5/#prefers-color-scheme

Platform coverage: All platforms

Estimated or target release: Firefox 67

Preference behind which this will be implemented:
The feature is always enabled, but there is an existing hidden
preference to set a value (integer), ui.systemUsesDarkTheme:
0 = light
1 = dark
2 = no-preference

Is this feature enabled by default in sandboxed iframes?
Yes, but if widget look-and-feel features are disabled in
sandboxed iframes then we'll match 'no-preference'.

DevTools bug: none

Do other browser engines implement this?
Safari has implemented this feature and it's available in their
Nightly build, but it's not yet shipped AFAICT.
https://paulmillr.com/posts/using-dark-mode-in-css/ is a post about
implementation of this feature in Safari;
https://webkit.org/blog/8475/release-notes-for-safari-technology-preview-68/ 
documents its addition.


It appears Chrome is actively working on it:
https://bugs.chromium.org/p/chromium/issues/detail?id=889087

web-platform-tests:
https://w3c-test.org/css/mediaqueries/prefers-color-scheme.html

Is this feature restricted to secure contexts? No

Credit also goes to Jonathan Kingston who wrote the initial
implementation of this feature.


/Mats


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


Re: Intent to implement: cryptomining and fingerprinting resource blocking

2019-04-01 Thread Ehsan Akhgari
Hi Rik,

Sorry for the late reply.

On Fri, Mar 22, 2019 at 10:10 PM Rik Cabanier  wrote:

>
>
> On Fri, Mar 22, 2019 at 6:07 AM Ehsan Akhgari 
> wrote:
>
>> On Thu, Mar 21, 2019, 9:39 PM Rik Cabanier,  wrote:
>>
>>> Why are these sites not included in the "safe browsing" service that is
>>> used by most browsers?
>>> That way, everyone would be protected.
>>>
>>
>> Because the relevant part of safe browsing service covers a different set
>> of criteria: https://www.google.com/about/unwanted-software-policy.html.
>>
>
> I think this page has the 3 criteria:
> https://safebrowsing.google.com/#policies
> It seems origins that try to fingerprint users or do cryptomining fall
> under category 1 and 3
>

Hmm, do you mean malware or social engineering?  I read
https://support.google.com/webmasters/answer/3258249 and
https://support.google.com/webmasters/answer/6350487/ and it is absolutely
not clear to me whether origins that fingerprint the user or engage in
(covert) cryptomining fall within those definitions.  The examples
mentioned on those two pages are specific to Chrome and other Google
software/policies and that's also unhelpful in determining whether those
categories include such software.

Note that I'm not saying that these categories are _not_ possibly a subset
of the existing SafeBrowsing database, but it's not obvious to us if they
are.

Another advantage that working on these categories with Disconnect will
have is that we will be able to document the technical details leading to
the decisions, for example see
https://github.com/disconnectme/disconnect-tracking-protection/blob/master/descriptions.md.
In the cases where I have (in my personal capacity) submitted a URL to
Google Safe Browsing, my experience has been that I have never heard back
about whether my submissions have been added to the database or not, and
what the reasons have been.

Cheers,
Ehsan


>
>
>> But more importantly, Google's safe browsing isn't by far the only block
>> list of bad URLs based on various criteria that various browsers and
>> extension use to improve the user's browsing experience. To answer your
>> actual question here, the block lists we're working with Disconnect to
>> create here are available for everyone to use under a permissive license at
>> https://github.com/disconnectme/disconnect-tracking-protection. We
>> actually ingest the list using the safe browsing protocol so other browsers
>> that have implemented that protocol could do the same today.
>>
>
> Good to know. Thanks for that link!
>
>
>>
>>> On Thu, Mar 21, 2019 at 2:59 PM Steven Englehardt <
>>> sengleha...@mozilla.com>
>>> wrote:
>>>
>>> > Summary:
>>> > We are expanding the set of resources blocked by Content Blocking to
>>> > include domains found to participate in cryptomining and
>>> fingerprinting.
>>> > Cryptomining has a significant impact on a device’s resources [0], and
>>> the
>>> > scripts are almost exclusively deployed without notice to the user [1].
>>> > Fingerprinting has long been used to track users, and is in violation
>>> our
>>> > anti-tracking policy [2].
>>> >
>>> > In support of this, we’ve worked with Disconnect to introduce two new
>>> > categories of resources to their list: cryptominers [3] and
>>> fingerprinters
>>> > [4]. As of Firefox 67, we have exposed options to block these
>>> categories of
>>> > domains under the “Custom” section of the Content Blocking in
>>> > about:preferences#privacy. We are actively working with Disconnect to
>>> > discover new domains that participate in these practices, and expect
>>> the
>>> > lists to grow over time. A full description of the lists is given here
>>> [5].
>>> >
>>> > Bugs:
>>> > Implementation: https://bugzilla.mozilla.org/show_bug.cgi?id=1513159
>>> > Breakage:
>>> > Cryptomining: https://bugzilla.mozilla.org/show_bug.cgi?id=1527015
>>> > Fingerprinting: https://bugzilla.mozilla.org/show_bug.cgi?id=1527013
>>> >
>>> > We plan to test the impact of blocking these categories during the
>>> Firefox
>>> > 67 release cycle [6][7]. We are currently targeting Firefox 69 to block
>>> > both categories by default, however this may change depending on the
>>> > results of our user studies.
>>> >
>>> > To further field test the new lists, we expect to enable the blocking
>>> of
>>> > both categories by default in Nightly within the coming month. If you
>>> do
>>> > discover breakage related to this feature, we ask that you report it
>>> in one
>>> > of the cryptomining or fingerprinting blocking breakage bugs above.
>>> >
>>> > Link to standard: These are additions to Content Blocking/Tracking
>>> > Protection which is not a feature we've standardized.
>>> >
>>> > Platform coverage:
>>> > Desktop for now. It is being considered for geckoview: (
>>> > https://bugzilla.mozilla.org/show_bug.cgi?id=1530789) but is on hold
>>> until
>>> > the feature is more thoroughly tested.
>>> >
>>> > Estimated release:
>>> > Disabled by default and available for testing in Firefox 67. We expect

Re: Intent to implement: Limit the maximum life-time of cookies set through document.cookie to seven days

2019-04-01 Thread Ehsan Akhgari
On Fri, Mar 22, 2019 at 1:55 PM  wrote:

> Hi Ehsan,
>
> Thanks for the follow up.
> I don't have access to a macOS computer with that 12.0.3 version of Safari
>
> On the other hand I have access to Google Analytics data for multiple
> sites, not the top 100 Alexa, but I don't see any evidence of a shift
> introduced by Safari 12.0.3 , for example through an increase in the share
> of new visitors with that version
>
> See that example https://photos.app.goo.gl/FTJoDLsYiJ17SPY46
>
> The Mozilla organization probably has access to similar data.
>
> Instrumentation could also be a good option to assess the impact of such
> type of change.
>

See
https://developers.google.com/analytics/devguides/collection/analyticsjs/cookie-usage#analyticsjs.
Google Analytics relies on two user identifier cookies, one expiring in 24
hours and one expiring in 2 years.  I'm not sure how the two identifiers
are linked together but it seems that they already treat repeat visitors
who come back to the site in less than 24 hours in a special way.  That may
explain why you haven't seen any impact as a result of this change.


> What I don't get is what is the assessment process in place to understand
> how such a change will be circumvented. Because it will. As many of the
> previous changes put in place by Safari ITP initiative.
>
> And the impact on the digital advertising ecosystem. The biggest ad
> networks might suffer. But will provide solutions, estimates,
> alternatives... And most of the smaller networks, with less agility, less
> money and resources, less skills, may break, get sold to the biggest, may
> close.
>

I'm not sure why you think this change will affect companies of different
sizes in a different way.  That is a strange proposition.

If you have any concrete worries please do share and I will be happy to
discuss.  Vague concerns like this usually read like the fear of the
unknown and are usually not helpful for having a productive discussion.


> Is the goal of this change really to enforce the role of the largest ad
> networks ? Why not. They tend to honor laws more likely than others...
>

It is not.  The goal of this change is to ensure that third-party scripts,
when running in the context of a top-level site, cannot abuse its cookie
jar to store cookies that can uniquely identify visitors and can be used to
keep a record of their browsing history across websites.  This change
achieves the goal by ensuring that such identifiers, once stored in the
top-level site's cookie jar, will expire in one week maximum, and therefore
the risk of the user being tracked by the companies that serve those
third-party scripts will be restricted to that window of time.


> Questioning anyway...
>
> This no just a technical decision about a web browser dropping an internet
> standard.
>
> What do you think ?
> What does the community thinks ?
>

If you have more feedback to share, this is the right place.  I would
recommend to keep the discussion focused on concrete technical issues
please and avoid FUD (
https://en.wikipedia.org/wiki/Fear,_uncertainty,_and_doubt) through
referencing claims such as dropping an internet standard or enforcing the
role of certain companies.  I would love to continue to have a discussion
around the technical merits of this feature.

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


SVG text coordinate systems

2019-04-01 Thread Alex Henrie
Hi, I am trying to fix Bug 1529182,[1] but debugging the SVG 
code is particularly difficult for me because it uses at least six
coordinate spaces, and I can't figure out how they relate to each
other:

- Run space
- Run user space
- Frame user space
- (Regular?) user space
- GFX space
- App space

Can anyone here explain them to me?

-Alex

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


Re: Improving our usage of Bugzilla

2019-04-01 Thread Kohei Yoshino

Due to unexpected circumstances, we couldn’t deploy and migrate the bug type 
field this week. Will try again next week as soon as on Monday.

Thanks,
-Kohei


On 2019-03-26 8:05 p.m., Kohei Yoshino wrote:

An update on the bug type and regression fields:

Because of Firefox 66 and the subsequent 66.0.1 pwn2own chemspill, we decided 
to delay the deployment of the change. We are now ready to deploy the new bug 
type and regressions/regressed-by fields to production Bugzilla later this week.

A one-time bug type auto migration will follow this weekend. It will be done 
with a component-based migration script [1] as well as the bugbug machine 
learning tool [2]. Marco tested bugbug with several bugs on the staging server 
last weekend and the results look good overall. The current instance can be 
seen on: https://bugzilla.allizom.org/

It should be noted that bugbug only classifies bugs as defect or enhancement 
over the last 2 years, because the accuracy of the detection between 
enhancement and task is not perfect. So triage owners and/or assignees will 
have to manually change the type of some existing bugs from enhancement to task 
once the migration is complete.

We’ll keep you posted.
-Kohei

[1] https://github.com/mozilla-bteam/bmo/pull/1106/files
[2] https://github.com/mozilla/bugbug


On 2019-03-12 6:55 a.m., Sylvestre Ledru wrote:

Bugzilla is one of the key tools used for the development of Firefox since the 
Netscape era. Even though this tool is serving us well, after interviewing a 
bunch of people at every level inside the company, we realized that we need to 
go the extra step.


Therefore, we decided to focus on improving Bugzilla itself but also, as a next 
step, improve the workflows we have on bugs.


To start with, we are coming with a set of three major changes to make our life 
better and easier.


1) Enforcing the usage of the priority field


As described in this schema 
,
 we are now asking triage owners to set the priority field according to the priority 
definition 
. The main 
goal is to make sure that every bug has been looked at and a priority has been set in 
accordance with the priority definitions.

A bot will automatically needinfo the triage owner (or a member of the team if 
it uses a round robin triage method).

We have been experimenting with this for several components and the results 
look great!


2) Bug type - new field

Firefox development requires a bug for every change in the Firefox code base. 
It doesn’t matter if this is used to fix a defect in the product, to implement 
a new feature or to refactor some code.


For years, we have been using bugzilla keywords to classify them. However, as 
they are not mandatory, the metadata can be missing, unmaintained, or 
inconsistent.


With this change, we are going to extend Bugzilla to add a new field with three 
new values:

  *

    Defect - an issue in one of our products

  *

    Enhancement - a new feature or an improvement

  *

    Task - a developer task. For example: refactor code foo.


Since we don’t want to increase the confusion for new users, the default value will 
be defectand, to the best of our ability, existing bugs will be moved 
under one of these 
categories.


With this information, we will be able to more precisely measure the quality of 
our products, as we will not mix defects together with feature-related work.


We have a development instance of Bugzilla 
where the changes can be evaluated, we are 
planning to go live in the new few weeks.


More information https://bugzilla.mozilla.org/1522340


3) Adding a new field called “Regressed by”

Currently, we misuse the blocks/depends fields to mark bugs causing 
regressions. However, they can be confusing and aren’t used consistently by 
developers. Moreover, since we are using bugs for defects, enhancements or 
tasks, it can be very hard to pinpoint the changes which introduced a 
regression.

Being able to pinpoint changes which introduced regressions will make it easier 
to build tools to help with assessing the riskiness of changes.


Therefore, we will add a new field in bugzilla which will clearly identify 
which bug caused a regression.

More:

http://bugzilla.mozilla.org/1461492


Last but not least, nothing is written in stone and we have other improvements 
(bug workflow, improved search, regression field, etc) coming. Sylvestre


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


Re: Improving our usage of Bugzilla

2019-04-01 Thread Kohei Yoshino

An update on the bug type and regression fields:

Because of Firefox 66 and the subsequent 66.0.1 pwn2own chemspill, we decided 
to delay the deployment of the change. We are now ready to deploy the new bug 
type and regressions/regressed-by fields to production Bugzilla later this week.

A one-time bug type auto migration will follow this weekend. It will be done 
with a component-based migration script [1] as well as the bugbug machine 
learning tool [2]. Marco tested bugbug with several bugs on the staging server 
last weekend and the results look good overall. The current instance can be 
seen on: https://bugzilla.allizom.org/

It should be noted that bugbug only classifies bugs as defect or enhancement 
over the last 2 years, because the accuracy of the detection between 
enhancement and task is not perfect. So triage owners and/or assignees will 
have to manually change the type of some existing bugs from enhancement to task 
once the migration is complete.

We’ll keep you posted.
-Kohei

[1] https://github.com/mozilla-bteam/bmo/pull/1106/files
[2] https://github.com/mozilla/bugbug


On 2019-03-12 6:55 a.m., Sylvestre Ledru wrote:

Bugzilla is one of the key tools used for the development of Firefox since the 
Netscape era. Even though this tool is serving us well, after interviewing a 
bunch of people at every level inside the company, we realized that we need to 
go the extra step.


Therefore, we decided to focus on improving Bugzilla itself but also, as a next 
step, improve the workflows we have on bugs.


To start with, we are coming with a set of three major changes to make our life 
better and easier.


1) Enforcing the usage of the priority field


As described in this schema 
,
 we are now asking triage owners to set the priority field according to the priority 
definition 
. The main 
goal is to make sure that every bug has been looked at and a priority has been set in 
accordance with the priority definitions.

A bot will automatically needinfo the triage owner (or a member of the team if 
it uses a round robin triage method).

We have been experimenting with this for several components and the results 
look great!


2) Bug type - new field

Firefox development requires a bug for every change in the Firefox code base. 
It doesn’t matter if this is used to fix a defect in the product, to implement 
a new feature or to refactor some code.


For years, we have been using bugzilla keywords to classify them. However, as 
they are not mandatory, the metadata can be missing, unmaintained, or 
inconsistent.


With this change, we are going to extend Bugzilla to add a new field with three 
new values:

  *

Defect - an issue in one of our products

  *

Enhancement - a new feature or an improvement

  *

Task - a developer task. For example: refactor code foo.


Since we don’t want to increase the confusion for new users, the default value will 
be defectand, to the best of our ability, existing bugs will be moved 
under one of these 
categories.


With this information, we will be able to more precisely measure the quality of 
our products, as we will not mix defects together with feature-related work.


We have a development instance of Bugzilla 
where the changes can be evaluated, we are 
planning to go live in the new few weeks.


More information https://bugzilla.mozilla.org/1522340


3) Adding a new field called “Regressed by”

Currently, we misuse the blocks/depends fields to mark bugs causing 
regressions. However, they can be confusing and aren’t used consistently by 
developers. Moreover, since we are using bugs for defects, enhancements or 
tasks, it can be very hard to pinpoint the changes which introduced a 
regression.

Being able to pinpoint changes which introduced regressions will make it easier 
to build tools to help with assessing the riskiness of changes.


Therefore, we will add a new field in bugzilla which will clearly identify 
which bug caused a regression.

More:

http://bugzilla.mozilla.org/1461492


Last but not least, nothing is written in stone and we have other improvements 
(bug workflow, improved search, regression field, etc) coming. Sylvestre

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


Intent to unship: CSP “require-sri-for” Support

2019-04-01 Thread Sebastian Streich
Summary:

In bug 1386214 we are planning to remove the Code for the "require-sri-for”
CSP directive.

The “require-sri-for” directive allows developers to block resource
requests that do not contain integrity metadata.

Please note that the entire code has always been behind a pref
(security.csp.experimentalEnabled) and we never shipped ‘require-sri-for’
by default.

Chrome also has flagged the feature as experimental and it seems they plan
to remove the code as well. See:
https://bugs.chromium.org/p/chromium/issues/detail?id=618924

We’re planning to remove the Feature in FF 69.


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

Link to standard: https://w3c.github.io/webappsec-subresource-integrity/



Thanks

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


Creating a new mach command for adding tests

2019-04-01 Thread 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: Creating a new mach command for adding tests

2019-04-01 Thread Jared Hirsch
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 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
>
___
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

2019-04-01 Thread Boris Zbarsky

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.



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.


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).


-Boris
___
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

2019-04-01 Thread Felipe G
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.

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.

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 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
> >
> ___
> dev-platform mailing list
> dev-pl

To what extent is sccache's distributed compilation usable?

2019-04-01 Thread Emilio Cobos Álvarez
Hi,

Over in #layout people were ranting about how much of a pain it was to
set up icecc on MacOS. I pointed out that sccache in theory does support
distributed compilation[1] and in theory also supports MacOS clients.

Does anybody know to what extent is it in a usable state? Can it replace
icecc? I'm a Linux user, and icecc works pretty well, but if sccache
supports Rust as well that's a huge plus.

Are people encouraged to try it out? Or only experimentally? Or should
we wait?

I know Ted encouraged me to try it in Berlin, and I still have to do
that (d'oh :/). So given that I suspect the answer is "it's worth trying
out and report bugs". But worth knowing if it's more generally usable or
mostly experimental.

Thanks,

 -- Emilio

[1]:
https://github.com/mozilla/sccache/blob/master/docs/DistributedQuickstart.md
___
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

2019-04-01 Thread 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


Re: To what extent is sccache's distributed compilation usable?

2019-04-01 Thread Bobby Holley
My (possibly outdated) understanding is that the sccache distributed
compilation stuff is restricted to the build machines we control. Naively,
I'm not sure there's a secure way to have machines in the wild contribute
build artifacts, given the risk of malicious code.

It seems plausible that we could at least allow read-only access to the
artifacts generated by the build machines, but that would presumably only
be useful if the developer's system configuration exactly matched that of
the build machine.

On Mon, Apr 1, 2019 at 12:23 PM Emilio Cobos Álvarez 
wrote:

> Hi,
>
> Over in #layout people were ranting about how much of a pain it was to
> set up icecc on MacOS. I pointed out that sccache in theory does support
> distributed compilation[1] and in theory also supports MacOS clients.
>
> Does anybody know to what extent is it in a usable state? Can it replace
> icecc? I'm a Linux user, and icecc works pretty well, but if sccache
> supports Rust as well that's a huge plus.
>
> Are people encouraged to try it out? Or only experimentally? Or should
> we wait?
>
> I know Ted encouraged me to try it in Berlin, and I still have to do
> that (d'oh :/). So given that I suspect the answer is "it's worth trying
> out and report bugs". But worth knowing if it's more generally usable or
> mostly experimental.
>
> Thanks,
>
>  -- Emilio
>
> [1]:
>
> https://github.com/mozilla/sccache/blob/master/docs/DistributedQuickstart.md
> ___
> 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

2019-04-01 Thread 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 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,
> > Bria

Re: Creating a new mach command for adding tests

2019-04-01 Thread 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

2019-04-01 Thread 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 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

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

Re: To what extent is sccache's distributed compilation usable?

2019-04-01 Thread Emilio Cobos Álvarez
On 01/04/2019 21:40, Bobby Holley wrote:
> My (possibly outdated) understanding is that the sccache distributed
> compilation stuff is restricted to the build machines we control. Naively,
> I'm not sure there's a secure way to have machines in the wild contribute
> build artifacts, given the risk of malicious code.
> 
> It seems plausible that we could at least allow read-only access to the
> artifacts generated by the build machines, but that would presumably only
> be useful if the developer's system configuration exactly matched that of
> the build machine.

Oh, the setup I was thinking of would be similar to how icecc works now,
on a local network. So, for example, N developers in the Berlin office
sharing a scheduler and build resources in the same network.

I'm not 100% sure whether the distributed compilation part of sccache is
only about sharing compilation artifacts, or also compiling stuff in
remote machines (like icecc does), or both, but afaik even in a local
network it could still be a pretty nice build-time / productivity
improvement.

 -- Emilio

> On Mon, Apr 1, 2019 at 12:23 PM Emilio Cobos Álvarez 
> wrote:
> 
>> Hi,
>>
>> Over in #layout people were ranting about how much of a pain it was to
>> set up icecc on MacOS. I pointed out that sccache in theory does support
>> distributed compilation[1] and in theory also supports MacOS clients.
>>
>> Does anybody know to what extent is it in a usable state? Can it replace
>> icecc? I'm a Linux user, and icecc works pretty well, but if sccache
>> supports Rust as well that's a huge plus.
>>
>> Are people encouraged to try it out? Or only experimentally? Or should
>> we wait?
>>
>> I know Ted encouraged me to try it in Berlin, and I still have to do
>> that (d'oh :/). So given that I suspect the answer is "it's worth trying
>> out and report bugs". But worth knowing if it's more generally usable or
>> mostly experimental.
>>
>> Thanks,
>>
>>  -- Emilio
>>
>> [1]:
>>
>> https://github.com/mozilla/sccache/blob/master/docs/DistributedQuickstart.md
>> ___
>> 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: To what extent is sccache's distributed compilation usable?

2019-04-01 Thread Chris M.
On Mon, Apr 1, 2019 at 12:23 PM Emilio Cobos Álvarez 
wrote:

> Hi,
>
> Over in #layout people were ranting about how much of a pain it was to
> set up icecc on MacOS. I pointed out that sccache in theory does support
> distributed compilation[1] and in theory also supports MacOS clients.
>
> Does anybody know to what extent is it in a usable state? Can it replace
> icecc? I'm a Linux user, and icecc works pretty well, but if sccache
> supports Rust as well that's a huge plus.
>
> Are people encouraged to try it out? Or only experimentally? Or should
> we wait?
>
Hi Emilio, if you're interested you're encouraged to try it out, however
we're shipping machines to offices now to run the schedulers, the first of
which I'll be testing in the SF office this week, so we're planning an
officially supported setup in the near future.

>
> I know Ted encouraged me to try it in Berlin, and I still have to do
> that (d'oh :/). So given that I suspect the answer is "it's worth trying
> out and report bugs". But worth knowing if it's more generally usable or
> mostly experimental.
>
> Thanks,
>
>  -- Emilio
>
> [1]:
>
> https://github.com/mozilla/sccache/blob/master/docs/DistributedQuickstart.md
> ___
> 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: To what extent is sccache's distributed compilation usable?

2019-04-01 Thread Nicholas Alexander
On Mon, Apr 1, 2019 at 1:03 PM Emilio Cobos Álvarez 
wrote:

> On 01/04/2019 21:40, Bobby Holley wrote:
> > My (possibly outdated) understanding is that the sccache distributed
> > compilation stuff is restricted to the build machines we control.
> Naively,
> > I'm not sure there's a secure way to have machines in the wild contribute
> > build artifacts, given the risk of malicious code.
> >
> > It seems plausible that we could at least allow read-only access to the
> > artifacts generated by the build machines, but that would presumably only
> > be useful if the developer's system configuration exactly matched that of
> > the build machine.
>
> Oh, the setup I was thinking of would be similar to how icecc works now,
> on a local network. So, for example, N developers in the Berlin office
> sharing a scheduler and build resources in the same network.
>
> I'm not 100% sure whether the distributed compilation part of sccache is
> only about sharing compilation artifacts, or also compiling stuff in
> remote machines (like icecc does), or both, but afaik even in a local
> network it could still be a pretty nice build-time / productivity
> improvement.
>

It does distribute compilation tasks to remote hosts.  At this time those
hosts must be running Linux (for the `bubblewrap` chroot-like package).
Months ago I configured this, following
https://github.com/mozilla/sccache/blob/master/docs/DistributedQuickstart.md,
for a Linux host and a macOS device.  Setting up the toolchains was
frustrating but do-able.  I did not get this working on the YVR office
build monster, just 'cuz I don't do a lot of native compilation.

Nick
___
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

2019-04-01 Thread Steve Fink

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


Re: Creating a new mach command for adding tests

2019-04-01 Thread 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: SVG text coordinate systems

2019-04-01 Thread Cameron McCormack
Hi Alex,

On Tue, Apr 2, 2019, at 1:47 AM, Alex Henrie wrote:
> Hi, I am trying to fix Bug 1529182,[1] but debugging the SVG 
> code is particularly difficult for me because it uses at least six
> coordinate spaces, and I can't figure out how they relate to each
> other:
> 
> - Run space
> - Run user space
> - Frame user space
> - (Regular?) user space
> - GFX space
> - App space
> 
> Can anyone here explain them to me?

Using this as an example:

  
  
abcd
  

the frame tree dump might look like this (with some unnecessary information 
elided):

  SVGOuterSVG(svg)(0) {0,0,24000,18000}
SVGOuterSVGAnonChild(svg)(0) {0,0,0,0}
  SVGText(text)(1) {5985,4875,8190,1410}
Block(text)(1) {0,0,2880,1440}
  line 7f2768ba20a8: count=2 {0,0,2880,1440}
Text(0)"ab" {0,30,1440,1380}
Inline(tspan)(1) {1440,30,1440,1380}
  Text(0)"cd" {0,0,1440,1380}

The curly braces are the mRect values for each frame, which are in app units, 
which are 1/60 of a CSS pixel.

SVGTextFrame works by leveraging the existing support for CSS rendering of 
blocks and inlines by building an nsBlockFrame for the  contents, 
nsInlineFrame for any , and nsTextFrame for text nodes.  This lets us 
avoid re-implementing text layout in the SVG code.  Anything SVG specific, like 
text positioning attributes or , are handled as a post-processing 
step when determining where to render each bit of the resulting CSS-positioned 
nsTextFrames.

Generally these mRects are relative to their parent, but the nsBlockFrame child 
of the SVGTextFrame is used just to lay out the text and is not rendered 
directly, so its (0, 0) position doesn't mean much.  Its width and height (2880 
x 1440 app units) are the result of reflowing it without any constraints on its 
size.  The layout of the block frame and its descendants is done independent of 
any SVG text positioning attributes and .

Because we leverage standard HTML/CSS rendering for SVG text, and because SVG 
text positioning attributes and s can affect the positions (and 
rotations) of individual glyphs, we produce TextRenderedRun objects to 
represent each contiguous sequence of characters within an nsTextFrame that we 
can ask the text frame to render in one go.  In this example, we end up 
producing three TextRenderedRuns:

* One for "a", at SVG text position (100, 100).  This covers {0,0,720,1380} of 
the first nsTextFrame.
* One for "b", at SVG text position (200, 200).  This covers {720,0,720,1380} 
of the first nsTextFrame.
* One for "cd", at SVG text position (200 + advance_in_user_units("b"), 200).  
This corresponds to the entire {0,0,1440,1380} of the second nsTextFrame.

Those rectangles might be in what you call "run space".  I don't think they're 
really named in the code.

In SVGTextFrame::PaintSVG, as we iterate over each TextRenderedRun, we need to 
set a transform on the gfxContext so that when asking an nsTextFrame to render 
some of its characters (the substring corresponding to the TextRenderedRun), it 
will appear in the right place.  Most of the complication in SVGTextFrame.cpp 
is doing this.

As for the coordinate systems you mention:

User space is the local SVG coordinate system for an SVG element.  In this 
example, the user space of the  element is the coordinate system the 
 establishes, with (0, 0) at the top level of the  element on the 
page, and (400, 300) at the bottom right.

"Run user space" is a term I made up to mean a coordinate space that is the 
same as the 's user space except translated so that the origin is at the 
top left of the rectangle that covers a given TextRenderedRun, and rotated by 
the TextRenderedRun's rotation.  This example doesn't have any rotate="" 
attributes or s, so there's no rotation here.  So in this coordinate 
system, (0, 0) is at the top left of the "b" glyph cell, and (12, 23) is at the 
bottom right of the "b" glyph cell.  TextRenderedRun::GetRunUserSpaceRect 
returns {0,0,12,23}, and TextRenderedRun::GetUserSpaceRect returns something 
like {200,86,12,23}, depending on the ascent/descent of the "b" (since the SVG 
text positioning aligns its alphabetic baseline at y=100).

"Frame user space" is something that doesn't really correspond to what you see 
on the page.  It's similar to "run user space": it's a coordinate system of the 
same scale as the 's user space, but with its origin placed at the top 
left corner of an nsTextFrame's rectangle.  For "b", 
TextRenderedRun::GetFrameUserSpaceRect returns {12,0,12,23} -- it's the same as 
GetRunUserSpaceRect but translated to where in its corresponding nsTextFrame is.

I'm not sure about "GFX space" and "app space" as terms.  There are many 
different units the coordinate values could be in -- see layout/base/Units.h.  
In SVG PaintSVG methods (e.g. SVGGeometryFrame::PaintSVG), setting `aTransform 
* aContext.CurrentMatrixDouble()` as the current matrix on the context will 
allow you to paint things in the current SVG element's u