Re: Should &&/|| really be at the end of lines?

[Emilio Cobos Álvarez]

On Wed, Feb 22, 2017 at 10:27:33AM -0500, Benjamin Smedberg wrote:
> On Thu, Feb 16, 2017 at 4:47 PM,  wrote:
> 
> > Question of the day:
> > When breaking overlong expressions, should &&/|| go at the end or the
> > beginning of the line?
> >
> > TL;DR: Coding style says 'end', I&others think we should change it to
> > 'beginning' for better clarity, and consistency with other operators.
> >
> 
> I will avoid having an opinion here, and merely state for the record that
> the final owner of this decision is Ehsan, as module owner of the C++/Rust
> style module [1]. Since the discussion appears to be winding down, I hope
> that Ehsan can make his decision known to finalize this email thread.

Oh, I didn't know we had a common module for both Rust and C++ style.

Given we do, I'd argue that probably it's a good idea to be consistent
across both, and the common Rust style is having those operators at the
end of lines[1].

 -- Emilio

[1]: 
https://github.com/rust-lang-nursery/fmt-rfcs/blob/1bd5b49dc38a48aaabe5d90c87e463351db42091/example/lists.rs#L302


signature.asc
Description: PGP signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Should cheddar-generated headers be checked in?

[Emilio Cobos Álvarez]

On Thu, Feb 23, 2017 at 08:25:30AM +0200, Henri Sivonen wrote:
> On Wed, Feb 22, 2017 at 5:49 PM, Ted Mielczarek  wrote:
> > Given that
> > the C API here is under your complete control, it seems like it's
> > possible to generate a cross-platform header
> 
> I believe the header is cross-platform, yes.
> 
> > Alternately you could just generate it at build time, and we could pass
> > the path to $(DIST)/include in a special environment variable so you
> > could put the header in the right place.
> 
> So just https://doc.rust-lang.org/std/env/fn.var.html in build.rs? Any
> naming conventions for the special variable? (I'm inferring from the
> way you said it that DIST itself isn't being passed to the build.rs
> process. Right?)

FWIW, in Stylo we use MOZ_DIST[1], which is passed to the build script,
not sure if it's stylo only though.

[1]: 
https://searchfox.org/mozilla-central/rev/b1044cf7c2000c3e75e8181e893236a940c8b6d2/servo/components/style/build_gecko.rs#48

> 
> -- 
> Henri Sivonen
> hsivo...@hsivonen.fi
> https://hsivonen.fi/
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform


signature.asc
Description: PGP signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Proper way to return nsresult from Rust before Stylo is mandatory?

[Emilio Cobos Álvarez]

On Fri, Mar 17, 2017 at 12:03:30PM +0200, Henri Sivonen wrote:
> It seems that our Rust bindings for nsresult are part of Stylo, but
> Stylo isn't yet a guaranteed part of the build.

FWIW those bindings are only checked-in into Servo for CI purposes (so
we can build the Gecko version of the style crate in Servo's CI without
requiring Gecko). The real bindings are generated at build-time.

> Until Stylo becomes a mandatory part of the build, what's the proper
> way to return nsresult from Rust such that it works with or without
> Stylo enabled?

We could try to split the compontents/style/gecko_bindings directory out
of stylo, I guess, though that'd require bindgen (and libclang) to be
enabled for everyone.

Also, we could do it all-at-once, which is a bit more risky because
bindgen had some build problems in some machines with some exotic C++
headers I'm trying to sort out, but we could also extract an nsresult
crate much like the nsString bindings, which should be pretty
straight-forward and shouldn't have those problems.

If everyone's fine with the libclang 3.9+ dependency, I guess it's fine
to do so. Otherwise we could checkout a static version of them, but
they'd need to be updated each time any error code changes.

Fedora didn't have libclang 3.9 packages IIRC, but that could've
changed.

 -- Emilio


signature.asc
Description: PGP signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Intent to unship: :-moz-bound-element pseudo-class.

[Emilio Cobos Álvarez]

I intend to remove the :-moz-bound-element CSS pseudo-class in bug
1350147.

It's an XBL-related pseudo-class that allows matching the XBL element
bound in the current subtree.

We could implement it in Stylo, I guess, but it is slightly annoying,
and seems completely unused and untested both in mozilla-central[1],
comm-central[2], and add-on code[3].

I don't think there's any benefit of keeping untested and unused stuff
in the tree, but if I got it wrong, or anyone has any concern, please
speak.

Thanks for reading :)

 -- Emilio

[1]: https://searchfox.org/mozilla-central/search?q=moz-bound-element
[2]: https://searchfox.org/comm-central/search?q=moz-bound-element
[3]: https://dxr.mozilla.org/addons/search?q=moz-bound-element&redirect=false


signature.asc
Description: PGP signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Using references vs. pointers in C++ code

[Emilio Cobos Álvarez]

Hi dev-platform@,

So, yesterday was working on a bug (bug 1362991, if you're curious) when
I decided to do some spring cleanup and pass some non-optional argument
as a reference instead of as a pointer.

I got the cleanup patch rejected, because it went against the prevailing
style of the codebase, and I was told that moving to references required
a discussion in dev-platform, resulting in something more explicit in
the style guide.

So, I'll revert that patch for now, but I'm sending this mail to
kickstart that discussion :)

TL;DR: I think references are useful. We have encoded nullability of
arguments and return values implicitly or explicitly in different ways:
assertions inside of the function, naming conventions...

I think references help to encode that a bit more in the type system,
and help reasoning about the code without having to look at the
implementation of the function you're calling into, or without having to
rely on the callers to know that you expect a non-null argument.

Personally, I don't think that the fact that they're not used as much as
they could/should is a good argument to prevent their usage, but I don't
know what's the general opinion on that.

Thanks for reading.

 -- Emilio


signature.asc
Description: PGP signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Using references vs. pointers in C++ code

[Emilio Cobos Álvarez]

On Tue, May 09, 2017 at 06:24:56PM +0300, smaug wrote:
> On 05/09/2017 04:52 PM, smaug wrote:
> > On 05/09/2017 01:55 PM, Mike Hommey wrote:
> > > On Tue, May 09, 2017 at 01:31:33PM +0300, Henri Sivonen wrote:
> > > > On Tue, May 9, 2017 at 12:58 PM, Emilio Cobos Álvarez 
> > > >  wrote:
> > > > > I think references help to encode that a bit more in the type system,
> > > > > and help reasoning about the code without having to look at the
> > > > > implementation of the function you're calling into, or without having 
> > > > > to
> > > > > rely on the callers to know that you expect a non-null argument.
> > > > > 
> > > > > Personally, I don't think that the fact that they're not used as much 
> > > > > as
> > > > > they could/should is a good argument to prevent their usage, but I 
> > > > > don't
> > > > > know what's the general opinion on that.
> > > > 
> > > > The relevant bit of the Core Guidelines is
> > > > https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rf-ptr-ref
> > > > and says:
> > > > "A pointer (T*) can be a nullptr and a reference (T&) cannot, there is
> > > > no valid "null reference". Sometimes having nullptr as an alternative
> > > > to indicated "no object" is useful, but if it is not, a reference is
> > > > notationally simpler and might yield better code."
> > > > 
> > > > As a result, I have an in-flight patch that takes T& instead of
> > > > NotNull where applicable, even though I do use NotNull to
> > > > annotate return values.
> > > > 
> > > > I agree that in principle it makes sense to use the type system
> > > > instead of relying on partial debug-build run-time measures to denote
> > > > non-null arguments when possible. That said, having to dereference a
> > > > smart pointer with prefix * in order to pass its referent to a method
> > > > that takes a T& argument feels a bit odd when one is logically
> > > > thinking of passing a pointer still, but then, again, &*foo seems like
> > > > common pattern on the Rust side of FFI to make a reference out of a
> > > > pointer and effectively asserting to the human reader that the pointer
> > > > is null.
> > > 
> > > Note that if you dereference a pointer to pass as a reference, all hell
> > > breaks loose and your reference might just as well be null, but the
> > > function taking the reference won't be protected against it because the
> > > compiler will have assumed, when compiling it, that the reference can't
> > > be null.
> > > 
> > This is what I'm a bit worried. We're moving some null checks from callee 
> > to caller, so need to
> > learn to be more careful with null checks on caller side.
> > But in general, using references sounds ok.
> > 
> 
> 
> I could add still, that using references does add another thing to reviewers' 
> checklist to ensure
> that null pointer isn't ever dereferenced, and it also means that there will 
> be more null checks, since
> parameters need to be validated on callers' side, not in callee.

To be clear, I'm not suggesting to move functions that now do the
null-checking in the function itself to take references. I think
functions that take pointers and handle null are perfectly fine.

But doing functions that already don't null-check because they're
already checked at the callsite seem to me they could perfectly take a
reference.

Also, there's a case for the opposite I think, where taking references
make reviewing easier.

To give one example, I just grepped a bit and found
EffectCompositor::GetAnimationElementAndPseudoForFrame[1] (there are a
lot of other functions like that, like all the IsFoo functions in the
frame constructor, etc).

In any case: That function dereferences the aFrame parameter without
null-checking, not even asserting.

If someone is, hypothetically, adding a caller to that function, and
calls it with a frame pointer that may be null, I think that function
taking a reference would, in any case, make the job of the reviewer
_easier_: It makes the explicit dereference at the callsite, so the
reviewer can evaluate whether that dereference is correct.

Otherwise, the reviewer would need to check the implementation of
GetAnimationElementAndPseudoForFrame in order to realise that it doesn't
handle null.

It's kind of a silly example (because that function itself doesn't make
a lot of sense if you don't give it a frame), but I can find some more
realistic examples of the same situation if you want to.

  -- Emilio

[1]: 
http://searchfox.org/mozilla-central/source/dom/animation/EffectCompositor.cpp#704


signature.asc
Description: PGP signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Using references vs. pointers in C++ code

[Emilio Cobos Álvarez]

On Tue, May 09, 2017 at 02:11:41PM -0700, L. David Baron wrote:
> On Tuesday 2017-05-09 11:58 +0200, Emilio Cobos Álvarez wrote:
> > I think references help to encode that a bit more in the type system,
> > and help reasoning about the code without having to look at the
> > implementation of the function you're calling into, or without having to
> > rely on the callers to know that you expect a non-null argument.
> > 
> > Personally, I don't think that the fact that they're not used as much as
> > they could/should is a good argument to prevent their usage, but I don't
> > know what's the general opinion on that.
> 
> I have two general concerns about references:
> 
>  (1) When reading the calling code that calls a function taking a
>  non-const reference, it's not obvious that the function might
>  mutate the value, particularly in code that uses references
>  rarely, or for types that are rarely passed as references.

Isn't it similarly non-obvious for code that takes non-const pointers
when the object isn't in the stack?

For example, if I have `void foo(Element*)`, which calls
`void bar(Element*)`, and both assume a non-null argument, I don't see
how potential mutation is more obvious there than `void foo(Element&)`
calling `void bar(Element&)`.

(Other people are concerned about non-const references used for
outparams. I think I'll be fine with the rule Nathan proposed in the
thread as a rule of thumb).

>  (2) Code that mixes functions that take references and functions
>  that take pointers, for the same time, is a mess, because you
>  constantly have to worry about whether to write * or &.

I agree with this, though I don't think writing & or * is a big deal in
most cases.

>  Thus, in Gecko, we generally use pointers or references per-type.
>  XPCOM interfaces are passed by pointer.  String classes are
>  passed by reference.  etc.
>
> I think per-type conventions for using references versus pointers
> helps with both (1) and (2), and I consider it part of Gecko style
> even if it's not well-documented.

The issue I have with per-type conventions is that it doesn't scale very
well, specially when you're writing new code (and more specially if
they're not documented in the style guide).

What should the convention be for `ServoStyleSet` (which is what
triggered this thread)? Who decides that, and based on which arguments?

> I'm also not sure how much references help with enforcing
> non-null-ness.  People will just write a "*" whether or not their
> existing code guarantees a non-null pointer; at least that was my
> experience in the early days of Gecko.  Perhaps we have better code
> review now... but I'm somewhat skeptical of that given that I think
> we've been relying on tests more and review less?

I'm not talking about enforcing non-null-ness, because as you've pointed
out, references don't necessarily do that.

I think the main benefit is at the time of both reading and reasoning
about the code. Using references allows you to quickly figure out what
kind of thing a function expects, handles, or returns, without having to
look at the function itself and potentially all the callers.

 -- Emilio

> -David
> 
> -- 
> 𝄞   L. David Baron http://dbaron.org/   𝄂
> 𝄢   Mozilla  https://www.mozilla.org/   𝄂
>  Before I built a wall I'd ask to know
>  What I was walling in or walling out,
>  And to whom I was like to give offense.
>- Robert Frost, Mending Wall (1914)




signature.asc
Description: PGP signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Using references vs. pointers in C++ code

[Emilio Cobos Álvarez]

On Tue, May 09, 2017 at 06:56:03PM -0700, L. David Baron wrote:
> On Wednesday 2017-05-10 02:43 +0200, Emilio Cobos Álvarez wrote:
> > The issue I have with per-type conventions is that it doesn't scale very
> > well, specially when you're writing new code (and more specially if
> > they're not documented in the style guide).
> > 
> > What should the convention be for `ServoStyleSet` (which is what
> > triggered this thread)? Who decides that, and based on which arguments?
> 
> In this particular case, I think it's easy.  ServoStyleSet is the
> Stylo version of a class (nsStyleSet) that uses pointer conventions
> because it used to implement XPCOM interfaces.  We shouldn't treat
> the two style set classes differently.  So it should be passed with
> pointers, not references.

I didn't know that nsStyleSet was previously an XPCOM interface. Does
that mean that stuff like nsStyleContext, etc shouldn't be
passed/returned by reference either in your opinion? (just realised it
was also an XPCOM interface at the time).

I think we shouldn't need to do code archeology in order to find out if
we can use references or not in the code.

 -- Emilio

> -David
> 
> -- 
> 𝄞   L. David Baron http://dbaron.org/   𝄂
> 𝄢   Mozilla  https://www.mozilla.org/   𝄂
>  Before I built a wall I'd ask to know
>  What I was walling in or walling out,
>  And to whom I was like to give offense.
>- Robert Frost, Mending Wall (1914)



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



signature.asc
Description: PGP signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: C++ performance test harness?

[Emilio Cobos Álvarez]

On 07/05/2017 10:19 AM, Henri Sivonen wrote:
> Do we have a gtest analog for local performance testing? That is,
> something that makes it easy to microbenchmark libxul methods?

For CSS parsing there is a benchmark using MOZ_GTEST_BENCH[1].

From grepping a bit seems like we also use it for testing stuff like
stripping whitespace on a string and such, is something like that what
you're looking for?

I'm not sure how easy it is (if possible at all) to test it on automation.

[1]:
http://searchfox.org/mozilla-central/source/layout/style/test/gtest/StyloParsingBench.cpp



signature.asc
Description: OpenPGP digital signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to unship: tree pseudo-element selectors from web content (::-moz-tree-*)

[Emilio Cobos Álvarez]

On 07/07/2017 06:03 AM, Xidorn Quan wrote:
> On Fri, Jul 7, 2017, at 01:42 PM, Jet Villegas wrote:
>> Thanks for cleaning this up.
>>
>> On Thu, Jul 6, 2017 at 8:29 PM, Xidorn Quan  wrote:
>>
>>> Although they don't currently match anything on web content, there is
>>> still some risk for unshipping them. Specifically, some websites seem to
>>> use them for browser-sniffing.
>>
>> Do you have some example sites that do this?
> 
> Yes, bug 1378846 is an example that a site doesn't render correctly in
> Stylo due to lack of support of that.

Would it be doable to parse them, but never match them? We do the same
for the :-moz-placeholder pseudo-class for similar reasons...

This would be slightly more tricky than just not parse them, at least
until we get rid of them internally (if we do), but would allow pages
using that kind of hacks to keep working.

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

Re: Care in the use of MOZ_CRASH_UNSAFE_* macros: data review needed

[Emilio Cobos Álvarez]

On 07/17/2017 05:18 PM, Benjamin Smedberg wrote:> Unlike MOZ_CRASH,
which only annotates crashes with compile-time constants
> which are inherently not risky, both MOZ_CRASH_UNSAFE_OOL and
> MOZ_CRASH_UNSAFE_PRINTF can annotate crashes with arbitrary data. Crash
> reasons are publicly visible in crash reports, and in general it is not
> appropriate to send any kind of user data as part of the crash reason.

I suppose the same happens with rust panics, should those also be reviewed?

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

Re: wpt CSS tests now running on Linux

[Emilio Cobos Álvarez]

Thanks for this James! \o/

One question, do we run the CSS test linter on automation, or are there
any plans for it?

We probably should, because otherwise we may only notice when trying to
upstream, like in [1], which is more work for everyone.

[1]:
https://github.com/w3c/web-platform-tests/pull/6357#issuecomment-311778265

On 07/20/2017 06:33 PM, James Graham wrote:
> Bug 1341078 and dependencies just landed on inbound, which means we now
> have the W3C/web-platform-tests CSS tests in-tree and running in
> automation. This adds about 12,000 reftests for CSS features to the
> web-platform-tests suite. They are currently enabled in CI, but only on
> linux*, due to limited capacity on OSX, and issues with the harness on
> Windows. The tests will be enabled on other platforms once these
> problems are resolved.
> 
> Servo was already running many of these tests in their automation, so
> this landing plugs a gap in the stylo testing vs upstream.
> 
> Note that, as usual with wpt landings, these tests have been vetted for
> stability, but not for the actual results.
> 
> Changes to the css tests in this directory will be upstreamed to
> web-platform-tests in the same way as for any other web-platform-test.
> Note that the reftest harness versions of the Mozilla submitted tests
> are still running, so if you want to edit or add to those it is
> recommended to use the copy in layout/reftests/w3c-css/submitted/ since
> that will be correctly synchronised and currently runs on more platforms
> in CI.
> 
> The number of tests and nature of reftests means that this change added
> a large number of files to the repository (around 37,000). Apologies for
> any inconvenience caused by such a large changeset. I'm told that narrow
> clones are just around the corner and may make this kind of thing more
> tolerable in the future.
> ___
> 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: Keyboard APZ has landed on Inbound

[Emilio Cobos Álvarez]

On 07/22/2017 08:05 AM, Ryan Hunt wrote:
> Keyboard APZ can't be used in every case. Currently it's disabled in the
> presense of key event listeners, as they can preventDefault scrolling and that
> is a non-negotiable part of the web.

Just curious, is there data about how often this happens? Does it affect
to key event listeners added to the document only, or do we disable this
also for other event listeners in any element?

Also, I assume this affects only to keypress events? Or also to other
kind of key events?

I would expect quite a few pages to use them...

> There may be issues with this enabled. Please test this out, and if you find
> any regressions please file bugs blocking bug 1352654.

Also, is bug 1352654 the right tracking bug? Doesn't seem related at a
glance.

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

Debugging Firefox e10s with rr?

[Emilio Cobos Álvarez]

Hi,

I didn't find any obvious docs in either the rr wiki[1] or MDN, so I
thought I'd ask before I actually need it.

What is the best/easiest way to debug Firefox multi-process using rr?

Right now I just disable e10s, but that's probably not a great long-term
solution...

 -- Emilio

[1]: https://github.com/mozilla/rr/wiki/Recording-Firefox
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Debugging Firefox e10s with rr?

[Emilio Cobos Álvarez]

On 08/29/2017 03:16 PM, Kartikaya Gupta wrote:
> Once you have a recording
> you can use `rr ps` to show all the process that were recorded and `rr
> replay -p ` to attach to a particular process.

I see, so `rr ps` was the bit whose existence I was missing :)

Thanks a lot!

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

Intent to unship: :-moz-system-metric pseudo-class and media queries in content pages.

[Emilio Cobos Álvarez]

Hi dev-platform@,

I'd like to unship access to the :-moz-system-metric pseudo-class, and
the system metric media queries, from content pages. I just filed
 for that.

They're not in any spec, and they seem unused (I can't find anything
non-mozilla-related in Github code search). Furthermore they expose
system information, which can be a fingerprinting vector, and pretty
random stuff like "is a color picker available?".

According to Boris it predates Gecko's capacity to not expose stuff.

I think it should be pretty safe to unship, but let me know if you don't
think that way, or have data that indicates the opposite.

The intention is to do that in Firefox 58, given we don't want to add
extra risk to Stylo.

Thanks!

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

Intent to unship: :-moz-empty-except-children-with-localname, :-moz-devtools-highlighted, and :-moz-styleeditor-transitioning from content documents.

[Emilio Cobos Álvarez]

Pretty much in the same spirit as my previous mail, I intend to unship
access to these pseudo-classes in content documents.

The idea would be, similarly, to do that for FF 58.

The reasoning behind is that there's no spec for them, and they're
implementation details, basically. In particular:

 * The devtools ones should be exposed only to UA sheets.
 * :-moz-empty-except-children-with-localname is only used for ,
is a super-slow selector, and we don't handle well dynamic changes to it
properly (nor is worth to try to do it, IMO).

I don't think I need to do a huge investigation to consider safe
unshipping these from content pages, but let me know if you think otherwise.

Feel free to reply here or in
, which I just
filed to this effect, if there are any concerns.

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

Re: Intent to unship: :-moz-system-metric pseudo-class and media queries in content pages.

[Emilio Cobos Álvarez]

On 09/02/2017 12:03 PM, Gijs Kruitbosch wrote:
> On 01/09/2017 20:06, Emilio Cobos Álvarez wrote:
> Can you elaborate on what you mean by "content pages" in this context?

In general I mean "exposed to web authors".

> I'm asking because I think the following things are true:
> 
> - Firefox UI is increasingly using in-content pages (ie content that
> loads in browser tabs) to "do stuff". This applies to network error
> pages, the preferences, the add-on manager, but also things you might
> spend less time thinking about like the "new tab" page.
> - From a security hardening perspective, we would like to avoid those
> pages having system principal as much as possible (ie ideally they are
> unprivileged, 'even' if they're using an about: URI)
> - Those pages quite regularly need to use OS-specific styling, and
> especially on Windows we sometimes do different things depending on
> whether the user is using a high contrast theme (which isn't something
> we currently expose to web content outside of the media queries etc.)
> - Even besides the browser pages that are using these, there are
> bindings we load on "normal" content pages (especially our default
> controls for  and ) that have styling that expects these
> media queries to work.

Yup, we already have special filtering to enable some stuff for chrome
URIs only. videocontrols.css is loaded from a chrome:// URI already, for
example, so I it has access to some features that normal web pages don't
have access to.

I suspect using the same filter would just work for the other use cases
you're mentioning (as long as the stylesheet is loaded from a chrome://
URI, we can expose stuff to it without exposing it to normal author
stylesheets).

Even if for some reason it wouldn't, it wouldn't be hard to make it work
for about: pages other than about:blank, for example (though I suspect
it's not going to be necessary to add the special-case).

> I remember that when we started doing some of this restricting for other
> parts of our CSS/Layout implementation, there were some issues with how
> we determine what counts as "content" and what counts as "chrome". How
> will that be done here, and will the usecases above continue to work?

I'm not aware of the background on this (I'd be curious to know about it
though). But as I said above I suspect the existing filtering mechanism
we already have will work, or can be made to work without much effort.

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

Coding style question: Meaningless argument names in declarations

[Emilio Cobos Álvarez]

Hi,

I have a coding style question which I don't see addressed in the coding
style page.

Given there have been a few threads lately on coding style, I guess I
should just ask.

We have a lot of meaningless argument names in function declarations, like:

  void DoFoo(Element& aElement);

Or:

  enum class Operation {
 // ..
  };

  void DoBar(Operation aOperation);

I personally think those argument names are mostly noise (the type gives
you the same information), and C++ allows omitting them.

That would make the signature more concise, like:

  void DoBar(Operation);

Which is helpful specially in long signatures.

I don't see anything mentioned in the style guide about this, should it be?

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

Re: Coding style question: Meaningless argument names in declarations

[Emilio Cobos Álvarez]

On 09/07/2017 04:01 PM, Andrew McCreight wrote:
> I don't know if it rises to the level of something that should be in the
> style guide, but I find the names very useful as a form of lightweight
> documentation. Sure, if you are naming your Foo arguments aFoo the names
> aren't useful, but for many widely used types like JSObject* and bool
> knowing whether the argument is the global or something else is very useful.

Sure, wasn't intending to suggest that we should say "no named arguments
in declarations".

This is very context dependent, I'd never omit a name in a function that
takes (Element& aParent, Element& aChild), but it seems reasonable to
omit it in the case where there's no better name, like (InsertionKind
aInsertionKind) or (Element& aElement).

My question is mostly if this is something people find acceptable.

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

Re: Intent to unship: Visibility of window.content to untrusted code

[Emilio Cobos Álvarez]

Just for the record, since I got curious and I saw no mention in the
intent email:

I've noticed that this may be used pretty easily for UA detection. So
far [1] is the only remotely related thing I've found from a search on
Google and GitHub (outside of the firefox codebase ofc).

I suspect keeping it exposed may cause more compat issues than removing
it, and given finding _something_ was super hard I suspect this is
pretty safe to remove, but if someone wants to take a closer look,
that'd also be great, I guess.

It'd have been great to have a counter on how many times the property is
accessed from a content doc or something, but I guess it may not be
totally representative, I've seen too much code iterating over the
window properties... :P

Anyway, great to remove another non-standard feature from content
documents :)

 -- Emilio

[1]: http://forums.mozillazine.org/viewtopic.php?f=25&t=232754

On 09/12/2017 09:32 PM, Boris Zbarsky wrote:
> Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=864845
> 
> window.content is a Gecko-specific thing that basically acts like
> window.top in untrusted code.  In chrome it returns the currently
> selected tab, effectively.
> 
> I would like to unship window.content for 57; no one else implements it.
> 
> -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: getComputedStyle now skips restyling if possible

[Emilio Cobos Álvarez]

On 09/28/2017 11:00 AM, Wei-Cheng Pan wrote:
> Hi,
> 
> This week I landed bug 1363805[1], which will skip restyling if the
> element does
> not need that for getting correct value. Normal users should not notice any
> difference, but this change may affect some test cases which need to force
> restyle.

Awesome! Thanks a lot for getting that landed, I know it was harder than
expected! :)

Just for the record, I happened to be looking at Blink code for
unrelated reasons, and found that we can probably optimize layout
flushes a bit better too.

I filed bug 1404140 with the details. I'll try to look at it, but I'm
probably not going to be able to fix it in the short-term. If you or
someone else can grab it sooner that would be absolutely lovely!

 -- Emilio

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

Re: Intent to implement: CSS media queries, interaction media features

[Emilio Cobos Álvarez]

On 10/03/2017 10:12 PM, Thomas Wisniewski wrote:> Security & Privacy
Concerns: this exposes whether the user has pointer that
> is finely controlled like a mouse, or more coarse-grained like a
> touchscreen (or no pointer at all). It also exposes whether they have a
> pointer capable of "hover" effects (like a mouse). This lets sites
> distinguish between the user having a mouse-like device or touchscreen-like
> device, and potentially other devices like stylii. It exposes a little more
> fingerprintable information than -moz-touch-enabled, however it would be
> trivial for anti-fingerprinting measures to always return a specific set of
> values for mobiles vs desktops.

Note that the system metric media queries respond to the
privacy.resistFingerprinting pref. I think we should keep doing the same
for this one.

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

#pragma once?

[Emilio Cobos Álvarez]

Hi,

I'm adding a header to the build, and I'm wondering: Can we use pragma once?

I don't see it anywhere in the build except third-party paths and:

  dom/svg/nsISVGPoint.h
  xpcom/io/nsAnonymousTemporaryFile.h

So I'm not sure if it's because it's somehow prohibited or not
recommended, or just because of inertia.

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

Re: Default Rust optimization level decreased from 2 to 1

[Emilio Cobos Álvarez]

On 10/25/2017 07:59 PM, Boris Zbarsky wrote:
> On 10/25/17 1:34 PM, Gregory Szorc wrote:
>> Also, due to ongoing work around Rust integration in the build system, it
>> is dangerous to rely on manual `cargo` invocations to compile Rust
>> because
>> bypassing the build system (not using `mach build`) may not use the same
>> set of RUSTFLAGS that direct `cargo` invocations do. Things were
>> mostly in
>> sync before. But this change and anticipated future changes will cause
>> more
>> drift. If you want the correct behavior, use `mach`.
> 
> Is something like "mach cargo-geckolib check" from within the servo/
> directory still expected to work sanely?

That's from the servo/ tree, right?

>From mozilla-central I've been using ./mach cargo check gkrust, and I
certainly expect it to keep working.

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

Intent to remove: elements matching hack

[Emilio Cobos Álvarez]

Hi,

In bug 1374247, I intend to remove the XBL compatibility hack introduced
in bug 653881 [1] for which  elements may be "transparent"
in selector-matching.

That means that a selector like .foo > .bar, would match a tree like:

  

  

  

The motivation is the following: This was not implemented in stylo, but
the Firefox UI depends on it, so we need to do something if we want
stylo on chrome documents.

I fear that it is the kind of thing that if we ever implement, it will
stay there forever. Also, one-offs like these tend to be buggy and
overlooked when implementing optimizations.

Finally, this hack doesn't apply to Shadow DOM, so removing it would
likely help with the efforts of transitioning away from XBL.

Since just removing it would have chances to silently break parts of the
Firefox UI, I'm landing a diagnostic assertion to verify that we don't
use it. I've been browsing for a couple days now with it without issues,
but if you get a crash pointing you to that bug, please file and I'll
take care of it :)

Thanks!

 -- Emilio

[1]:
https://hg.mozilla.org/mozilla-central/rev/1b8207252e9ca1194eccd3adc14b961785fb4e8e
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Intent to unship: as in image maps

[Emilio Cobos Álvarez]

Hi,

In bug 1317937 I intend to unship the feature of  elements acting the
same way as  elements in image maps.

This functionality was specced in HTML 4, but no other browser
implemented it and was removed from HTML 5.

Timothy (:tnikkel) tried to do it before, but it got blocked on getting
telemetry for this (bug 1350532).

Given that didn't advance for 8 months, that it blocks (or at least
simplifies a lot) longstanding bug 135040, which always bites again and
is a nice source of FIXME comments[1], and the fact that this is not
implemented anywhere else and is not in any spec anymore, we think it's
reasonable to just remove it, and we don't expect any webcompat fallout
from it.

Let me know if there's any concern about doing this.

 -- Emilio

[1]: http://searchfox.org/mozilla-central/search?q=135040
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Intent to unship: mozmm CSS unit.

[Emilio Cobos Álvarez]

Hi,

In bug 1416564 I intend to remove the mozmm CSS unit.

This unit is Mozilla-only, has no spec, and is unused in all our
codebase (except for two tests, one of those which tests the unit itself).

This unit was introduced experimentally in bug 537890, our browser
chrome code used it in bug 588464, and all that is gone since then.

Given there's no spec, and no usage in the wild as far as I can tell, I
think we should try to remove it.

Thoughts?

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

Re: Intent to unship: mozmm CSS unit.

[Emilio Cobos Álvarez]

On 11/12/2017 05:14 PM, Xidorn Quan wrote:
> IIRC, we have discussed unshipping this somewhere before we added its
> support to stylo (maybe the Taipei meeting this year?) and dbaron said
> that mozmm provides an ability to size something based on physical size
> which is not directly possible in any other unit, so this is something
> we may want to put into spec rather than removing.

Huh, I don't recall being part of that discussion, but that sounds
plausible.

> I doubt if there is anything changed since then (except that we still
> haven't pushed this on csswg), so maybe we still shouldn't remove it.

I'm definitely ok with adding something like that if there's an use-case
and demand for it... But given I haven't been able to find any actual
usage of it, not even internal, I suspect there isn't much?

In any case, having features exposed to the web without any
specification or other browsers supporting it looks like the wrong thing
to me, and the kind of thing that may bite in the future (and that makes
Servo's life harder, too), so I still would like to unship it if possible.

If afterwards we figure out that this is needed, I'd be happy to see it
added as something every browser agrees with. My patch keeps the ability
to convert physical millimeters to CSS pixels, so it'd be trivial to
reintroduce if Mozilla decides to push for this feature and other
browsers agree.

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

Re: Intent to unship: mozmm CSS unit.

[Emilio Cobos Álvarez]

Also, another thought I just had about this unit: Probably if we don't
unship it, then privacy.resistFingerPrinting should do something about
it, since it allows to calculate the DPI of the screen trivially using
CSSOM accessors.

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

Re: Intent to unship: mozmm CSS unit.

[Emilio Cobos Álvarez]

Hi David,

On 11/15/2017 02:27 AM, L. David Baron wrote:
> On Sunday 2017-11-12 16:11 +0100, Emilio Cobos Álvarez wrote:
>> Hi,
>>
>> In bug 1416564 I intend to remove the mozmm CSS unit.
>>
>> This unit is Mozilla-only, has no spec, and is unused in all our
>> codebase (except for two tests, one of those which tests the unit itself).
>>
>> This unit was introduced experimentally in bug 537890, our browser
>> chrome code used it in bug 588464, and all that is gone since then.
>>
>> Given there's no spec, and no usage in the wild as far as I can tell, I
>> think we should try to remove it.
>>
>> Thoughts?
> 
> So I think this unit had a pretty strong use case:  designing of
> touch user interfaces, where touch targets need to be at least a
> certain physical size in order to work well with human fingertips.
> 
> There's also a risk from having physical units in CSS that we
> learned the last time CSS had them:  designs that work with some
> ratios of physical units to other units and break with other ratios.
> 
> I've been meaning to dig up the minutes from the time we tried to
> get the CSS working group to add these units, but I haven't had a
> chance over the last few days.  I think if those minutes suggest
> that the working group was receptive to adding such units to the
> spec (or perhaps even agreed to do so, but then they were never
> actually added by the editor), I'd tend to think we should leave our
> implementation, whereas if the minutes suggest that the balance of
> working group opinion was against them, then we should remove it.
> Would you be able to try searching for those minutes?  I know
> there's been an in-person discussion (and I think it was at a
> meeting at a TPAC a few yeas ago).

So, I've looked through and I haven't found any related minutes (I've
looked for minutes that mentioned "physical", of which they were many,
but all related to physical properties and scroll snapping, and for
minutes that mentioned mozmm, of which they were none).

I've found two threads in the w3c mailing list that look related, one
which is basically the discussion that lead to all units being changed
to be relative to the density of the display:

  https://lists.w3.org/Archives/Public/www-style/2010Jan/0058.html

And someone interjecting on a thread about adding the Q unit:

  https://lists.w3.org/Archives/Public/www-style/2013Nov/0302.html

Then there's a relevant, more recent, csswg-drafts issue:

  https://github.com/w3c/csswg-drafts/issues/614

But from the comments in there it doesn't seem to be a strong sense
about it being useful, but mostly the opposite (and actually [1] may be
a legit bug we should look into if we keep it in tree, or behavior we
should document). Also, some other alternatives for that use case that
work in all browsers are mentioned, like resolution-dependent media queries.

What do you think David? I landed the patch this morning, and I'd really
prefer it to stick. Given the recent cssswg-drafts issue where the CSSWG
members were not really excited about it, I think it's ok to remove it,
but happy to back the patch out if you think it's not.

 -- Emilio

[1]: https://github.com/w3c/csswg-drafts/issues/614#issuecomment-254679777

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

Re: Intent to unship: mozmm CSS unit.

[Emilio Cobos Álvarez]

On 11/15/2017 11:51 AM, Jonathan Kew wrote:
> On 15/11/2017 03:32, Emilio Cobos Álvarez wrote:
> 
>> So, I've looked through and I haven't found any related minutes (I've
>> looked for minutes that mentioned "physical", of which they were many,
>> but all related to physical properties and scroll snapping, and for
>> minutes that mentioned mozmm, of which they were none).
>>
>> I've found two threads in the w3c mailing list that look related, one
>> which is basically the discussion that lead to all units being changed
>> to be relative to the density of the display:
>>
>>    https://lists.w3.org/Archives/Public/www-style/2010Jan/0058.html
>>
>> And someone interjecting on a thread about adding the Q unit:
>>
>>    https://lists.w3.org/Archives/Public/www-style/2013Nov/0302.html
> 
> It sounds like you may have missed the lengthy "[css3-values] Physical
> length units" thread (spun off from "[css3-mediaqueries] DPI in
> resolution media queries") that started in mid-Feb 2012:

Indeed, thanks Jonathan! I bet the minutes at

  https://lists.w3.org/Archives/Public/www-style/2012Feb/0530.html

were what David was referring to.

>   https://lists.w3.org/Archives/Public/www-style/2012Feb/0627.html
> 
> I haven't re-read all that thread to see whether anything resembling a
> consensus seemed to be emerging my (vague) recollection is that
> there's generally little enthusiasm in the WG for a "true" physical
> unit, with perhaps one or two vocal dissenters
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Fennec/Android turns on Quantum CSS (stylo) as default

[Emilio Cobos Álvarez]

On 11/22/2017 05:15 PM, Tom Ritter wrote:
> On Wed, Nov 22, 2017 at 9:51 AM, Jet Villegas  wrote:
>> Do you have a use case for shipping the ESR with --disable-stylo? We want to
>> be very quick about removing the legacy C++ style system as it adds
>> significant impedance to new feature development. I have not heard of any
>> site breakage that would warrant keeping --disable-stylo after Stylo is in
>> Fennec.
> 
> Without --disable-style, Tor hits
> https://bugzilla.mozilla.org/show_bug.cgi?id=1390583 and is in a bad
> position.
> 
> Hope is to have this fixed by ESR 59, but it's a difficult issue to
> work through.

Have you tried recently? There's a bunch of work that happened in bug
1341234 and related bugs, which fix similar issues to the ones you were
having I think.

I'm happy to help on bindgen's side if there are fixes needed, same for TB.

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

Re: Intent to implement (again): Shadow DOM

[Emilio Cobos Álvarez]

On 11/27/2017 02:01 PM, James Graham wrote:
> On 27/11/17 12:20, smaug wrote:
>> This is basically an after the fact notification that
>> we're in progress of implementing Shadow DOM again, this time v1[1].
>> While doing this, the v0 implementation, which was never exposed to
>> the web, will be removed.
>> v1 is luckily way simpler, so this all should simplify various bits in
>> DOM.
>>
>> FF60 might be quite realistic target to ship this, but there will be
>> intent-to-ship
>> before that.
>>
>> Many parts of the spec (DOM) is stable, but there are still couple of
>> tricky issues in HTML, like
>> session history handling with shadow DOM. However Chrome and Safari
>> are shipping v1 already.
>>
>> Devtools will be able to see into the Shadow DOM.
>>
>> Currently the work is under the old pref "dom.webcomponents.enabled"
>> but perhaps we should change that, so that people who were testing v0
>> don't get
>> v1 APIs.
> 
> Do we have cross-browser (i.e. web-platform) tests for this feature, and
> have we assessed whether they are sufficiently complete to give us
> confidence of interop?

There are some web platform tests, but I'm moderately sure that the
rendering bits of them are not quite enough, since they're quite basic.

I also know that Chromium and WebKit have tons of crashtests and tests
for dynamic changes of the page that we'd definitely benefit from.

I know there's an effort in Chromium to upstream LayoutTests to WPT. The
Shadow DOM v1 LayoutTests would be extremely helpful to have upstream IMO.

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

Re: Intent to implement (again): Shadow DOM

[Emilio Cobos Álvarez]

On 11/28/2017 10:44 AM, Philip Jägenstedt wrote:
> On Mon, Nov 27, 2017 at 2:50 PM Emilio Cobos Álvarez  <mailto:emi...@crisal.io>> wrote:
> 
> On 11/27/2017 02:01 PM, James Graham wrote:
> > On 27/11/17 12:20, smaug wrote:
> >> This is basically an after the fact notification that
> >> we're in progress of implementing Shadow DOM again, this time v1[1].
> >> While doing this, the v0 implementation, which was never exposed to
> >> the web, will be removed.
> >> v1 is luckily way simpler, so this all should simplify various
> bits in
> >> DOM.
> >>
> >> FF60 might be quite realistic target to ship this, but there will be
> >> intent-to-ship
> >> before that.
> >>
> >> Many parts of the spec (DOM) is stable, but there are still couple of
> >> tricky issues in HTML, like
> >> session history handling with shadow DOM. However Chrome and Safari
> >> are shipping v1 already.
> >>
> >> Devtools will be able to see into the Shadow DOM.
> >>
> >> Currently the work is under the old pref "dom.webcomponents.enabled"
> >> but perhaps we should change that, so that people who were testing v0
> >> don't get
> >> v1 APIs.
> >
> > Do we have cross-browser (i.e. web-platform) tests for this
> feature, and
> > have we assessed whether they are sufficiently complete to give us
> > confidence of interop?
> 
> There are some web platform tests, but I'm moderately sure that the
> rendering bits of them are not quite enough, since they're quite basic.
> 
> I also know that Chromium and WebKit have tons of crashtests and tests
> for dynamic changes of the page that we'd definitely benefit from.
> 
> I know there's an effort in Chromium to upstream LayoutTests to WPT. The
> Shadow DOM v1 LayoutTests would be extremely helpful to have
> upstream IMO.
> 
> 
> What kind of timeline are we talking about here? Are any of the tests
> more valuable than others, such that you'd import them into Gecko
> directly from Chromium if they aren't upstreamed to wpt?

Yeah, so.. The timeline is FF60, so it'd be before March 2018 if I got
the math right.

And yeah, I plan to prioritize the tests. The most interesting ones for
me are the CSS / rendering ones... In case this isn't in the timeline
for the Chromium exports, I'd try to first export to WPT myself, or
otherwise import into Gecko (but ideally the first).

I'm happy to help out exporting them or what not if needed.

 -- Emilio

> +Hayato Ito <mailto:hay...@chromium.org> FYI.
> 
> (Reposted after subscribing.)
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Intent to unship -moz-border-*-colors from content pages.

[Emilio Cobos Álvarez]

Hi dev-platform@,

In bug 1417200, I intend to hide the -moz-border-top-colors,
-moz-border-bottom-colors, -moz-border-left-colors, and
-moz-border-right-colors from content pages.

The reasons for doing this are multiple:

 * Non-standard properties.

 * Kinda-weird, in the sense that the "border" shorthand resets it, but
the other border- shorthands don't.

 * Quoting jrmuizel:

> Supporting this is a pain for webrender. It would be nice not to
expose them to web content and then eventually remove from chrome.

 * Typical use cases can be addressed with border-image in a
cross-browser way.

 * It got a standard replacement in the latest draft of css-backgrounds
(making border-color take a list of colors instead of just one), so if
there's buy-in from other vendors we can expose that functionality
there, see [1] and [2].

I think it's nicer to remove these before other browsers implement that,
because otherwise we may be stuck with them forever, or make removing
these much more risky.
Thoughts?

 -- Emilio

[1]: https://github.com/w3c/csswg-drafts/issues/1172#issuecomment-295565255
[2]: https://drafts.csswg.org/css-backgrounds-4/#propdef-border-color
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Intent to unship: @-moz-document from content pages.

[Emilio Cobos Álvarez]

Hi again,

In bug 1035091 I intend to remove support for the @-moz-document CSS
rule in content pages (more exactly in author stylesheets).

The reasoning for this, apart from it being a non-standard mozilla-only
CSS feature, is that it's a possible security risk in presence of CSS
injection attacks (Freddy, CCd, can give more details about this, there
are also some links on the bug).

Since it's a useful feature for user stylesheets, we're keeping it on those.

Let me know if there's any concern on doing this.

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

Re: Intent to unship: @-moz-document from content pages.

[Emilio Cobos Álvarez]

On 11/29/2017 06:36 PM, Mike Taylor wrote:
> 
>> On Nov 29, 2017, at 10:53 AM, Emilio Cobos Álvarez  wrote:
>>
>> In bug 1035091 I intend to remove support for the @-moz-document CSS
>> rule in content pages (more exactly in author stylesheets).
> 
> This is a pretty widely used mechanism to target styles for Gecko. Would it 
> be possible to disable in non-release for a few releases to sniff out any 
> major layout/compat bustage?

Sure, sounds good.

Another thing we could try to do if plain unshipping fails would be to
just hide the regex matching function, which IIUC would prevent the
security issue too. But hiding it behind a pref on non-release for now
sounds good.

Boris, would you also be fine with that?

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

Re: Firefox build issues with Rust and the new VS2017 15.5 update

[Emilio Cobos Álvarez]

On 12/05/2017 05:16 PM, Ryan VanderMeulen wrote:
> FYI, the VC++ 2017 v14.12 toolset included in the recently-released VS2017
> 15.5 update appears to have broken building Firefox due to issues with the
> Rust compiler (in particular, the version of libclang we ship with it) and
> one of the system headers:
> 
> C:\PROGRA~2\MIB055~1\2017\COMMUN~1\VC\Tools\MSVC\1412~1.258\include\type_traits:898:47:
> error: '_Ty' does not refer to a value
> 
> Which in turns leads to a Rust panic and build failure.

That looks like bindgen, but the error is a C++ one, that is, it's clang
who is choking to parse type_traits. Which version of libclang is this
using?

There's nothing tying the rust compiler version and the libclang
version, so probably just using a newer clang that is up-to-date with
the C++ features that that MSVC header uses should be enough.

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

Re: Intent to implement: individaul transform

[Emilio Cobos Álvarez]

On 12/15/2017 08:51 AM, Ku(顧思捷)CJ wrote:
> Summary:
>   The translate, rotate, and scale properties allow authors to specify
> simple transforms independently, in a way that maps to typical user
> interface usage, rather than having to remember the order in transform that
> keeps the actions of transform(), rotate() and scale() independent and
> acting in screen coordinates.
>   Both Blink and Edge have implemented this feature.
> 
> Bug:
>   https://bugzilla.mozilla.org/show_bug.cgi?id=1207734
> 
> Link to standard:
>   https://drafts.csswg.org/css-transforms-2/#individual-transforms
> 
> Platform coverage:
> All platforms
> 
> Target release:
>   FF60
> 
> Preference behind which this will be implemented:
>   "layout.css.individual-transform.enabled"
> 
> Do other browser engines implement this?
>   Blink/ Edge

Just curious, has Edge shipped this? Blink still hasn't. Not that it
makes much of a difference, but I guess it's worth noting.

> Tests:
>   WPT test
> ___
> 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: Next year in web-platform-tests

[Emilio Cobos Álvarez]

On 12/16/2017 04:57 AM, L. David Baron wrote:
> On Friday 2017-12-15 16:00 -0600, Andrew McCreight wrote:
>> For me, a blocker to only having WPT is leak checking, both in terms of
>> XPCOM leak checking and LeakSanitizer. (The latter is probably going to
>> automatically work if you run them in ASan, but it would be good to check.)
>> I know there's a bug for XPCOM leak checking open already.
> 
> There are also some other pieces that our existing test harnesses
> (reftest and mochitest) do but the WPT harnesses don't, such as
> assertion checking (in a way that allows us to mark existing
> NS_ASSERTIONs as known failures, and get reports of unexpected
> changes in either direction, without losing the rest of the
> capability of the test).  These make me somewhat hesitant to port
> even existing reftests that can be ported to use WPT (see bug
> 1263568 for previous discussion of these issues), particularly in
> areas where assertions have been useful at catching problems.  For
> example, I'd be hesitant to add new reftests for css-multicol
> directly to a wpt directory rather than to a directory that is run
> by the layout/tools/reftest harness since assertion checking is
> particularly important for multicol tests.

Probably not all of them may be workable over the next year, but
assertions are the main thing blocking bug 1425167, so if that could be
sorted out it'd be great.

Otherwise, I guess I can just make a bunch of assertions fatal, which
may be the right thing to do anyway.

James, do you know if there's a bug on file to track these? Otherwise I
can file.

 -- Emilio

> There are also a bunch of other features of the reftest harness
> documented in layout/tools/reftest/README.txt that aren't part of
> WPT that have been important for testing various things related to
> painting, layers, async pan/zoom, scrolling, printing, etc., that
> are things that simply can't be done in WPT.
> 
> -David
> 
> 
> 
> ___
> 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: support CSS paint-order for HTML text

[Emilio Cobos Álvarez]

On 12/24/2017 02:01 PM, Jonathan Kew wrote:
> Tests - Is this feature fully tested by web-platform-tests? No, as it is
> not yet spec'd (see above). I propose to land a basic mozilla reftest
> along with the patches in bug 1426146 (behind a pref); if/when the CSS
> WG agrees to accept this issue in the spec, we can migrate the reftest
> to WPT

Just FYI, other people land tests into WPT with .tentative.html in the
name, like:

  https://github.com/w3c/web-platform-tests/pull/8602

Not sure what's preferred, I believe that if the chances of this getting
spec'd are high it may be better, but...

James, do you know whether there's any official guideline for these kind
of situations?

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

Re: How to use Gecko in my web browser as web engine

[Emilio Cobos Álvarez]

I think probably dev-platform is a better list to get an answer
regarding Gecko embedding, rather than dev-tech-layout, so posting there.

I wish I could be more helpful, but I know nothing about that :).

 -- Emilio

On 1/31/18 8:23 AM, malay emailme wrote:
> Dear Sir,
>I want to build a web browser in qt and i want gecko as my
> default web engine.Please guide me how to use.
>  from
> malay
> ___
> dev-tech-layout mailing list
> dev-tech-lay...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-tech-layout
> 
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to ship: Array.prototype.values

[Emilio Cobos Álvarez]

On 02/02/2018 03:27 PM, Andrew Overholt wrote:
> On Fri, Feb 2, 2018 at 8:45 AM, Tom Schuster  > wrote:
> 
> Chrome seems to
> want to add a kill pref for this,  from my experience more difficult
> for us with the way we define methods in SpiderMonkey. Should that be
> a requirement for shipping?
> 
> 
> If it's not too difficult it does make it a lot easier to fix things if
> it causes major breakage. I think we can even do a pref flip without
> shipping an update which is pretty nice.
> 
> Is there a way to do per-site pref changes? Did we do this for Stylo,
> Emilio?

We did, but it required a fair amount of machinery. The code for the
stylo domain blocklist was removed very recently in bug 1426223.

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

Re: Correct way to build style bindings on GNU/Linux ARMv7?

[Emilio Cobos Álvarez]

On 02/07/2018 06:04 PM, Henri Sivonen wrote:
> mach bootstrap doesn't appear to download a bindgen-compatible clang
> on an ARMv7 GNU/Linux host.
> 
> mach build then complains about not being able to generate the stylo bindings.
> 
> As I understand it, --disable-stylo-build-bindgen should be OK if one
> isn't modifying stylo, but building with that flag fails:

It fails because the position of those flags is different depending on
the architecture:


https://searchfox.org/mozilla-central/rev/84cea84b12145d752e50ddca6be5462c38510e35/dom/base/nsWrapperCache.h#31

So it's not the bindings what are failing. If your purpose is building
(not running), you can probably just tweak the wrapper.rs file to remove
the conditional #[cfg]s:


https://searchfox.org/mozilla-central/rev/84cea84b12145d752e50ddca6be5462c38510e35/servo/components/style/gecko/wrapper.rs#190

Hope it helps,

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

Re: gkrust compilation RAM requirements and 32-bit systems

[Emilio Cobos Álvarez]

On 02/09/2018 10:49 AM, Henri Sivonen wrote:
> Is there some trick to make gkrust compilation succeed on a 32-bit system?

The BSD folks seem to be using --disable-debug-symbols for that, see
https://bugzilla.mozilla.org/show_bug.cgi?id=1401093

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

Commit messages in Phabricator

[Emilio Cobos Álvarez]

Hey,

So I've been testing Phabricator for a while and I've noticed that the 
arc tool uses different formatting for the commit messages than what 
usually lands on mozilla-central.


In particular I usually lay out my commits like:

---
Bug XXX: Short summary. r=

Long description, if needed.
---

Arc wants to use something like:

---
Short summary.

Summary:
Long description (I assume?).

Test Plan: (what goes here?)

Reviewers: 

Subscribers: (what goes here?)

Bug #: XXX

Differential revision: https://phabricator.services.mozilla.com/D574
---

I've been manually fixing them up before committing them so they have 
the usual format when landing.


My question is: Are commits that are going to land to autoland from 
phabricator directly going to land with the new format?


Should we keep fixing them up before pushing to autoland / inbound, or 
should we start pushing commits with the new commit message formatting?


Also, knowing an answer for the (what goes here?) questions would be 
super-helpful.


Thanks a lot!

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

Intent to unship: -moz-user-input: disabled / enabled

[Emilio Cobos Álvarez]

Hi,

In bug 1405087 I intend to remove the "enabled" and "disabled" values of 
the -moz-user-input property.


The reasoning for this is because they're slightly broken in presence of 
dynamic changes (see the bug), and their usage seems low / web-compat 
risk seems not too high.


In particular, I could only a couple legit uses of -moz-user-input: 
disabled that weren't either commented out or broken (using 
"moz-user-input" instead of "-moz-user-input"). Of those uses, all of 
them were part of the jQuery iButton plugin:


  https://www.givainc.com/labs/ibutton_jquery_plugin.cfm

Which I've verified works fine with my patches.

-moz-user-input: enabled works exactly the same as -moz-user-input: 
auto, which is the default, and thus only changes behavior if it's 
specified under something that has -moz-user-input: disabled / none.


Again, only legit use case of this I found (within a -moz-user-input: 
disabled / none element) was the mentioned jQuery plugin, which works 
just fine without those.


If we happen to find web-compat fallout from this we can consider 
re-introducing them / aliasing them to the other values, I guess, and 
just remove our internal usage, but I think it's worth trying to remove 
them in light of the above.


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

Re: Intent to unship: -moz-user-input: disabled / enabled

[Emilio Cobos Álvarez]

On 02/14/2018 06:00 PM, L. David Baron wrote:

On Wednesday 2018-02-14 09:25 +0100, Emilio Cobos Álvarez wrote:

Hi,

In bug 1405087 I intend to remove the "enabled" and "disabled" values of the
-moz-user-input property.


Do other browser engines implement the 'user-input' property
(prefixed or unprefixed)?  Which values do they support?


No other browser implements this property as far as I can tell, either 
prefixed or unprefixed. I've tested Edge 16 (via BrowserStack), WebKit 
Nightly via Epiphany's preview, and Chromium 63.



The original spec for this property was:
   https://www.w3.org/TR/2000/WD-css3-userint-2216#user-input
which seems to only have:
   Values: none | enabled | disabled | inherit

So it sounds like this would leave 'auto' and 'none', where 'auto'
wasn't part of the original spec and 'none' was.


I'd be happy to try removing the property entirely, but that seemed more 
risky since we have at least a bunch of internal usage for the "none" value.


That being said, most of that internal usage comes from patches mentioning:

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

Which I'm not sure is relevant at all these days, probably not.

That usage comes mostly from EditorOverride.css and contenteditable.css.

If someone familiar with editor could take a look and confirm that 
they're not relevant anymore, we could try to remove the property entirely.


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

Intent to unship: Legacy -moz-transform syntax.

[Emilio Cobos Álvarez]

Hi,

In bug 1438297 I intend to remove the legacy syntax for -moz-transform, 
which allows lengths or percentages apart from numbers in some transform 
matrix components.


That will make -moz-transform a normal alias for the "transform" 
property, just like -webkit-transform, following the usual parsing rules.


The reasoning for this is that this particular property is the only 
alias that requires different parsing rules from the property they're 
aliasing to. That means it needs to be implemented as a shorthand 
property, which means in turn a bunch of special-cases to serialize it, etc.


I think the risk for this should be minimal given these properties are 
unprefixed from a long time ago and we keep the property working with 
the syntax that works on all browsers (plus we also have -webkit- 
prefixes). Just as a data point, we didn't have a single reftest that 
failed without it, only parsing tests.


Let me know if you think otherwise though.

Note also that there's bug 770560 for unprefixing them completely, I'm 
more interested on removing the complexity from the code for now so I 
haven't investigated that yet, so I think this change is less risky.


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

Intent to unship: ::-moz-math-anonymous.

[Emilio Cobos Álvarez]

Hi,

In bug 1439837 I intend to remove access to the ::-moz-math-anonymous 
pseudo-element.


This is a Gecko-only pseudo-element that is there since the initial 
Gecko export, and is only used to get a style inheriting from another 
one for some MathML characters, so it has no good reason to be exposed 
to the web at all.


I'm not aware of anything or anyone using it apart from us.

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

Re: Intent to unship: ::-moz-math-anonymous.

[Emilio Cobos Álvarez]

On 02/21/2018 07:02 PM, Tantek Çelik wrote:

SGTM. I did not find any references on MDN, so nothing to update there AFAIK.

However with a broader web search I found https://gist.github.com/yields/6648240

Is ::-moz-math-anonymous special? (last of its kind?) Or would the
same reasoning apply to removing access to ::-moz-math-stretchy or any
of the others in that list? Or is that just an out-of-date list that
we can ignore?


Looks out of date to me. ::-moz-math-stretchy was removed four years ago 
in https://bugzilla.mozilla.org/show_bug.cgi?id=1000879.



Thanks,

Tantek

P.S. I also found moz-math-anonymous on
http://mdn.beonex.com/en/CSS_Reference/Mozilla_Extensions.html#Pseudo-elements_and_pseudo-classes
but that seems to just be an out-of-date copy of
https://developer.mozilla.org/en-US/docs/Web/CSS/Mozilla_Extensions#Pseudo-elements_and_pseudo-classes
which already removed the -moz-math- pseudos.


On Wed, Feb 21, 2018 at 4:30 AM, Emilio Cobos Álvarez  wrote:

Hi,

In bug 1439837 I intend to remove access to the ::-moz-math-anonymous
pseudo-element.

This is a Gecko-only pseudo-element that is there since the initial Gecko
export, and is only used to get a style inheriting from another one for some
MathML characters, so it has no good reason to be exposed to the web at all.

I'm not aware of anything or anyone using it apart from us.

  -- Emilio
___
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: Intent to unship: ::-moz-math-anonymous.

[Emilio Cobos Álvarez]

On 02/21/2018 09:13 PM, Frédéric Wang wrote:

On 21/02/2018 19:07, Emilio Cobos Álvarez wrote:


On 02/21/2018 07:02 PM, Tantek Çelik wrote:

SGTM. I did not find any references on MDN, so nothing to update there
AFAIK.

However with a broader web search I found
https://gist.github.com/yields/6648240

Is ::-moz-math-anonymous special? (last of its kind?) Or would the
same reasoning apply to removing access to ::-moz-math-stretchy or any
of the others in that list? Or is that just an out-of-date list that
we can ignore?


Looks out of date to me. ::-moz-math-stretchy was removed four years ago
in https://bugzilla.mozilla.org/show_bug.cgi?id=1000879.


Hi Emilio,

IIRC, ::-moz-math-stretchy was the only MathML pseudo element that was
publicly documented but it's no longer needed now that people can just
use font-family and/or the font preference menu to select a math font.
It should be safe to stop exposing -moz-math-anonymous and
-moz-mathml-anonymous-block, if we do.


Thanks for confirming that Fred!

Just for the record, ::-moz-mathml-anonymous-block is already not 
exposed, since it's an anonymous box. This looks to be the case since 
https://bugzilla.mozilla.org/show_bug.cgi?id=377824.


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

Re: skip-if(verify)

[Emilio Cobos Álvarez]

On 03/06/2018 06:04 PM, Geoffrey Brown wrote:
> It is now possible to skip tests in test-verify. Simplify annotate the
> manifest for your test:
> 
> [test]
> skip-if = verify
> 
> or, for reftests:
> 
> skip-if(verify) ...
> 
> and the test-verify (TV) test task will not try to verify the annotated
> test.
> 
> Please don't abuse this feature! Most TV failures indicate a weakness in
> the test.

Could you point to an example of a "good" use of this feature? Is it
just to avoid TV failing too intermittently? Is TV unable to run some
tests? When does TV provide no value?

Thanks!

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

Re: skip-if(verify)

[Emilio Cobos Álvarez]

On 03/06/2018 08:05 PM, Kyle Machulis wrote:
> In my experience, many tests were written pre-test-verify, and don't clean
> up correctly to deal with multiple runs in the same process. They work fine
> when running as a single session, but blow up in TV. Having skip-if(verify)
> means that we can at least mark those tests as known broken on TV without
> taking them out of the test suites completely, especially if we find them
> in places that we don't know how to fix them and need to file to someone
> else who may not currently have time to fix.

Ahá, that makes perfect sense, thanks a lot!

 -- Emilio

> On Tue, Mar 6, 2018 at 10:49 AM, Emilio Cobos Álvarez 
> wrote:
> 
>> On 03/06/2018 06:04 PM, Geoffrey Brown wrote:
>>> It is now possible to skip tests in test-verify. Simplify annotate the
>>> manifest for your test:
>>>
>>> [test]
>>> skip-if = verify
>>>
>>> or, for reftests:
>>>
>>> skip-if(verify) ...
>>>
>>> and the test-verify (TV) test task will not try to verify the annotated
>>> test.
>>>
>>> Please don't abuse this feature! Most TV failures indicate a weakness in
>>> the test.
>>
>> Could you point to an example of a "good" use of this feature? Is it
>> just to avoid TV failing too intermittently? Is TV unable to run some
>> tests? When does TV provide no value?
>>
>> Thanks!
>>
>>  -- Emilio
>> ___
>> 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: Intent to unship: @-moz-document from content pages.

[Emilio Cobos Álvarez]

On 11/29/17 6:36 PM, Mike Taylor wrote:
> 
>> On Nov 29, 2017, at 10:53 AM, Emilio Cobos Álvarez  wrote:
>>
>> In bug 1035091 I intend to remove support for the @-moz-document CSS
>> rule in content pages (more exactly in author stylesheets).
> 
> This is a pretty widely used mechanism to target styles for Gecko. Would it 
> be possible to disable in non-release for a few releases to sniff out any 
> major layout/compat bustage?

Just for completeness, we did find breakage (see dependencies of that
bug). I fixed most of those, and Youtube fixed theirs on their side.

All of it was related to @-moz-document url-prefix(), so even though I'd
still like to eventually get rid of it, for now I've added a pref:

  layout.css.moz-document.url-prefix-hack.enabled

which controls whether @-moz-document url-prefix() parses or not.

The intention is that for pre-release builds there's no change (no
@-moz-document in content at all) since we still want to eventually flip
that pref, but for release we'll ship:

  layout.css.moz-document.content.enabled = false;
  layout.css.moz-document.url-prefix-hack.enabled = true;

That is, pages with @-moz-document url-prefix() { foo } will keep
working, but not other matching function like regex().

Let me know if there's any concern with doing this.

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

Re: Intent to unship: @-moz-document from content pages.

[Emilio Cobos Álvarez]

On 3/19/18 11:21 AM, Emilio Cobos Álvarez wrote:
> On 11/29/17 6:36 PM, Mike Taylor wrote:
>>
>>> On Nov 29, 2017, at 10:53 AM, Emilio Cobos Álvarez  wrote:
>>>
>>> In bug 1035091 I intend to remove support for the @-moz-document CSS
>>> rule in content pages (more exactly in author stylesheets).
>>
>> This is a pretty widely used mechanism to target styles for Gecko. Would it 
>> be possible to disable in non-release for a few releases to sniff out any 
>> major layout/compat bustage?
> 
> Just for completeness, we did find breakage (see dependencies of that
> bug). I fixed most of those, and Youtube fixed theirs on their side.
> 
> All of it was related to @-moz-document url-prefix(), so even though I'd
> still like to eventually get rid of it, for now I've added a pref:
> 
>   layout.css.moz-document.url-prefix-hack.enabled
> 
> which controls whether @-moz-document url-prefix() parses or not.
> 
> The intention is that for pre-release builds there's no change (no
> @-moz-document in content at all) since we still want to eventually flip
> that pref, but for release we'll ship:
> 
>   layout.css.moz-document.content.enabled = false;
>   layout.css.moz-document.url-prefix-hack.enabled = true;
> 
> That is, pages with @-moz-document url-prefix() { foo } will keep
> working, but not other matching function like regex().
> 
> Let me know if there's any concern with doing this.

Oh, missed it, this is tracked in bug 1446470.

> 
>  -- Emilio
> ___
> 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 ship: OpenType Variation Font support

[Emilio Cobos Álvarez]

On 03/20/2018 11:22 AM, Jonathan Kew wrote:> There are a handful of
tests now in
> web-platform/tests/css/css-fonts/variations, and there a bunch more
> currently in preparation (e.g. see
> https://bugzilla.mozilla.org/show_bug.cgi?id=1436588).

There's also https://github.com/w3c/web-platform-tests/pull/9373. Do you
know how interoperable are we on those tests? They seem to have caught
at least a few Blink and Edge compat issues.

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

PSA: No longer needed to annotate tests differently for stylo-disabled platforms.

[Emilio Cobos Álvarez]

In bug 1446954 I'm removing those platforms from our automation in
preparation to removing the old code, now that stylo is enabled everywhere.

This mostly affects you if you're hacking on the style system itself, or
in Shadow DOM stuff, which only works on the new style system.

There's probably a fair amount of WPT expectations and mochitest cleanup
that we can do. I went through the reftests in that same bug. Help on
that would be lovely :)

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

Intent to unship: CSSStyleDeclaration.getPropertyCSSValue

[Emilio Cobos Álvarez]

Hi,

Bug 1408301 tracks unshipping CSSStyleDeclaration.getPropertyCSSValue.

This is a non-standard API only implemented by Mozilla, and that
generally can be replaced by usage of the standard .getPropertyValue.

We added a use counter and deprecation warning in bug 474655. The data
seems to indicate the usage is low, so I'm trying to remove this API on
Nightly only to see if there's any breakage in bug 1448415, tentatively
under the pref layout.css.getPropertyCSSValue.enabled.

Let me know if there's any concern with doing this, and please file bugs
blocking either bug 1448415 or bug 1408301 if you see something broken
because of this.

Thanks!

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

PSA: --disable-stylo is gone.

[Emilio Cobos Álvarez]

Following [1], the code for the old style system has been removed[2][3],
and cleanups started[4][5].

There's tons of easy-ish cleanups that you're welcome to help with if
you want, see bugs blocked by https://bugzil.la/stylo-everywhere.

The --disable-stylo build option is gone[6], and thus you may need to
update your mozconfig.

Apologies in advance if you used it as a way to get faster build times
by building less Rust code :). If you're really annoyed by it there's
some low-hanging fruit to do[7] on our side that I'm happy to mentor /
help with.

Cheers,

 -- Emilio

[1]:
https://groups.google.com/d/msg/mozilla.dev.platform/Xw43_tGMPM0/3hgouOp-AAAJ
[2]: https://bugzil.la/1447358
[3]: https://bugzil.la/1447367
[4]: https://bugzil.la/1447476
[5]: https://bugzil.la/1447483
[6]: https://bugzil.la/1447611
[7]: https://bugzil.la/1412441
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

PSA: --disable-stylo is gone

[Emilio Cobos Álvarez]

Following my last message[1], the code for the old style system has been
removed (bug 1447358, bug 1447367), and cleanups started (bug 1447367,
bug 1447476, ...).

There's tons of easy-ish cleanups that you're welcome to help with if
you want, see bugs blocked by https://bugzil.la/stylo-everywhere.

The --disable-stylo build option is gone (bug 1447611), and thus you may
need to update your mozconfig.

Apologies in advance if you used it as a way to get faster build times
by building less Rust code . If you're really annoyed by it there's some
low-hanging fruit to do on our side, like bug 1412441, that I'm happy to
mentor / help with.

Cheers,

 -- Emilio

[1]:
https://groups.google.com/d/msg/mozilla.dev.platform/Xw43_tGMPM0/3hgouOp-AAAJ
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

PSA: --disable-stylo is gone.

[Emilio Cobos Álvarez]

Following [1], the code for the old style system has been removed[2][3],
and cleanups started[4][5].

There's tons of easy-ish cleanups that you're welcome to help with if
you want, see bugs blocked by https://bugzil.la/stylo-everywhere.

The --disable-stylo build option is gone[6], and thus you may need to
update your mozconfig.

Apologies in advance if you used it as a way to get faster build times
by building less Rust code :). If you're really annoyed by it there's
some low-hanging fruit to do[7] on our side that I'm happy to mentor /
help with.

Cheers,

 -- Emilio

[1]:
https://groups.google.com/d/msg/mozilla.dev.platform/Xw43_tGMPM0/3hgouOp-AAAJ
[2]: https://bugzil.la/1447358
[3]: https://bugzil.la/1447367
[4]: https://bugzil.la/1447476
[5]: https://bugzil.la/1447483
[6]: https://bugzil.la/1447611
[7]: https://bugzil.la/1412441
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to unship: CSSStyleDeclaration.getPropertyCSSValue

[Emilio Cobos Álvarez]

On 03/23/2018 11:50 PM, Jonathan Watt wrote:
> On 23/03/2018 18:23, Emilio Cobos Álvarez wrote:
>> Bug 1408301 tracks unshipping CSSStyleDeclaration.getPropertyCSSValue.
>>
>> This is a non-standard API only implemented by Mozilla
> 
> It was removed from Blink[1] after they forked, but it's actually still
> implemented in Webkit it seems. Hopefully we shouldn't have too many problems
> removing it though.

Hmm, I should trust less our code comments[1], I guess the "sort of" is
key. :P

Looks like WebKit does indeed implement it, also on specified values (we
only expose it on the computed style declaration). But yeah, given
neither Blink or Edge implement it, and that what we implement is widely
different than what WebKit implements (that Blink thread is a good
summary) sounds unlikely that it cause compat pain.

[1]:
https://searchfox.org/mozilla-central/rev/56274d0a016a6833e5da7770ce70580b6f5abb21/dom/webidl/CSSStyleDeclaration.webidl#22

> 
> Jonathan
> 
> 1. 
> https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/3VmxWFzcyJc
> 
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Intent to unprefix: ::-moz-selection.

[Emilio Cobos Álvarez]

Hi,

In bug 509958 I intend to unprefix the ::-moz-selection pseudo-element.

The situation here is not great wrt the spec saying what we do, or what
other implementations do for that matter, see [1].

However other engines have shipped this unprefixed for a long time with
the same semantics that we implement, and we're seeing webcompat
problems due to people forgetting to include ::-moz-selection in their
style sheets, see bug 1427680 or bug 1448670 for some examples.

Given I don't think other engines have any incentive to implement what
the spec says (given it's slower, and has the chance of giving them
webcompat headache), and that we're compatible with them, I proposed to
adapt the spec to reality in that spec issue, and unprefix our
pseudo-element.

Let me know if there's any concerns with this plan. Thanks!

 -- Emilio

[1]: https://github.com/w3c/csswg-drafts/issues/2474
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to unprefix: ::-moz-selection.

[Emilio Cobos Álvarez]

On 3/26/18 4:13 PM, Boris Zbarsky wrote:

On 3/26/18 3:16 AM, Emilio Cobos Álvarez wrote:

However other engines have shipped this unprefixed for a long time with
the same semantics that we implement


https://bugzilla.mozilla.org/show_bug.cgi?id=509958#c14 claims the 
semantics are not actually quite the same.  Have we done some testing 
here?  Including on Edge?


So, when I went through the bug I didn't know at all what that comment 
was about (and the account that posted it was gone so couldn't ask), but 
it seems we don't respect the ::selection background when selecting images.


That looks like an easy fix though, I'll ensure it gets fixed before 
landing. Filed bug 1449010.


It would be best to just have matching 
semantics, instead of unprefixing with slightly different behavior.


Haven't tested Edge (because I don't have it locally), but will make 
sure that gets done before landing that patch.


 -- Emilio


-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: Intent to unprefix: ::-moz-selection.

[Emilio Cobos Álvarez]

On 03/28/2018 12:09 AM, twisniew...@mozilla.com wrote:
> On Tuesday, March 27, 2018 at 4:38:56 PM UTC-4, Emilio Cobos Álvarez wrote:
>> That looks like an easy fix though, I'll ensure it gets fixed before 
>> landing. Filed bug 1449010.
>  
> If it helps, I had a patchset for this in bug 292563, but ran into an odd 
> reftest failure that I never had time to figure out.

Ah, I see, interesting!

I used negative reftests (!=) to test this, instead of just trying to
overlay the image, which looks a little bit more fragile.

Given my patch just landed, I'll dupe that bug. Thanks!

 -- Emilio

> ___
> 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 ship: Allow the overflow shorthand to accept two values.

[Emilio Cobos Álvarez]

Hi,

In bug 1453148 I'm planning to implement the CSSWG resolution at:

  https://github.com/w3c/csswg-drafts/issues/2484

It's a very uncontroversial change that doesn't change backwards compat,
and makes the shorthand more consistent. But it was probably worth an
intent.

WPT tests are being added as part of that bug. Let me know if there's
any concern with proceeding here, thanks!

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

Coding style: brace initialization syntax

[Emilio Cobos Álvarez]

Sorry, I know, coding style thread... But it's Friday and this is 
somewhat related to the previous thread.


Bug 525063 added a lot of lines like:

explicit TTextAttr(bool aGetRootValue)
  : mGetRootValue(aGetRootValue)
  , mIsDefined{ false }
  , mIsRootDefined{ false }
{
}

I think I find them hard to read and ugly.

Those changes I assume were generated with clang-format / 
clang-format-diff using the "Mozilla" coding style, so I'd rather ask 
people to agree in whether we prefer that style or other in order to 
change that if needed.


Would people agree to use:

 , mIsRootDefined { false }

Instead of:

 , mIsRootDefined{ false }

And:

 , mFoo { }

Instead of:

 , mFoo{}

?

I assume the same would be for variables, I find:

  AutoRestore restore { mFoo };

easier to read than:

  AutoRestore restore{ mFoo };

What's people's opinion on that? Would people be fine with a more 
general "spaces around braces" rule? I can't think of a case right now 
where I personally wouldn't prefer it.


Also, we should probably state that consistency is preferred (I assume 
we generally agree on that), so in this case braces probably weren't 
even needed, or everything should've switched to them.


Finally, while I'm here, regarding default member initialization, what's 
preferred?


  uint32_t* mFoo = nullptr;

Or:

  uint32_t* mFoo { nullptr };

I'm ambivalent, but brace syntax should cover more cases IIUC (that is, 
there are things that you can write with braces that you couldn't with 
equals I _think_).


Should we state a preference? Or just say that both are allowed but 
consistency is better?


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

Re: Coding style: brace initialization syntax

[Emilio Cobos Álvarez]

On 4/13/18 4:22 PM, Boris Zbarsky wrote:
So my take is that we should not use braced initializer syntax in 
constructor initializer lists.  The reason for that is that it makes it 
much harder to scan for where the constructor body starts.


I don't think that's true in the general case where the braces remain to 
the right of the identifier in the same line, fwiw, but...



Doubly so when ifdefs in the initializer list are involved. > Triply so when 
someone writes it as:

   explicit TTextAttr()
     , mIsRootDefined
   {
     false
   }
#ifdef SOMETHING
#endif
   {
   }

which is what clang-format did in some of the cases in the patch for bug 
525063.


... this is definitely terrible I agree :(


In particular, I think this should have just used:

   , mIsRootDefined(false)


Agreed in this case (specially given there were previous members 
initialized with parenthesis, and the braces are not really necessary).


I don't have a strong opinion about the one space when we do use the 
braced initializer syntax.  But we should make sure we don't end up with 
the above monstrosity where it looks like the ctor body.


I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1453973 for that.


-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: Coding style: brace initialization syntax

[Emilio Cobos Álvarez]

On 4/13/18 4:49 PM, Nathan Froyd wrote:

On Fri, Apr 13, 2018 at 9:37 AM, Emilio Cobos Álvarez  wrote:

Those changes I assume were generated with clang-format / clang-format-diff
using the "Mozilla" coding style, so I'd rather ask people to agree in
whether we prefer that style or other in order to change that if needed.

Would people agree to use:

  , mIsRootDefined { false }

Instead of:

  , mIsRootDefined{ false }

What's people's opinion on that? Would people be fine with a more general
"spaces around braces" rule? I can't think of a case right now where I
personally wouldn't prefer it.


If we are going to have brace-initialization intermixed with
list-initialization (i.e. parentheses) in our codebase, I think we
should prefer no space prior to the brace, for consistency. 


Hmm, consistency with parenthesis I guess, but not with other things 
that use braces, like conditionals or other kind of declarations (where 
we use a space after the paren, for example), or lambdas where we use 
`mutable {`, etc.


Also, I guess per that argument we should use `mIsRootDefined{false}` 
instead of `mIsRootDefined{ false }`.



If we are going to switch wholesale (which would be a big job!)... I'd
probably say "no space", just on "that's the way we've always done it"
grounds, but can be convinced otherwise.


Though this is a good point though, I've only found a couple of them 
with spaces:


  $ rg ' m[A-Z][A-Za-z]* \{ ' | wc -l 



  9

vs.

  $ rg ' m[A-Z][A-Za-z]*\{ ' | wc -l
  516

Though that could be an artifact of clang-format in the directories we 
run it.



I agree with bz on disallowing braces in constructor init lists.


I'd be ok with that I guess, though it's more common each time? Also, is 
there any case where you could use braces but not parenthesis? (I'm not 
a C++ expert in this regard).



Also, we should probably state that consistency is preferred (I assume we
generally agree on that), so in this case braces probably weren't even
needed, or everything should've switched to them.


Indeed.


Finally, while I'm here, regarding default member initialization, what's
preferred?

   uint32_t* mFoo = nullptr;

Or:

   uint32_t* mFoo { nullptr };


I lean towards the former here.  I think the former is more common in
the code I've seen, but apparently the latter is "preferred C++" or
something?


I think the former is ok, but wouldn't work for stuff like:

  Atomic mFoo = nullptr;

While:

Atomic mFoo { nullptr };

does work.

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

Re: Default Rust optimization level decreased from 2 to 1

[Emilio Cobos Álvarez]

There's a fair amount of people bitten by this constantly, which see 
long style profiling markers and what's really happening is that they're 
profiling a local opt build, and thus the Rust code in style has barely 
any optimization and is slow.


I know that shouldn't be a thing, and that people should 
--enable-release for profiling and all that. But given it happens, could 
we consider reverting this change?


 -- Emilio

On 10/25/17 7:34 PM, Gregory Szorc wrote:

Compiling Rust code with optimizations is significantly slower than
compiling without optimizations. As was measured in bug 1411081, the
difference between rustc's -Copt-level 1 and 2 on an i7-6700K (4+4 cores)
for a recent revision of mozilla-central was 325s/802s wall/CPU versus
625s/1282s. This made Rust compilation during Firefox builds stand out as a
long pole and significantly slowed down builds.

Because we couldn't justify the benefits of level 2 for the build time
overhead it added, we've changed the build system default so Rust is
compiled with -Copt-level=1 (instead of 2).

Adding --enable-release to your mozconfig (the configuration for builds we
ship to users) enables -Copt-level=2. (i.e. we didn't change optimization
settings for builds we ship to users.)

Adding --disable-optimize sets to -Copt-level=0. (This behavior is
unchanged.)

If you want explicit control over -Copt-level, you can `export
RUSTC_OPT_LEVEL=` in your mozconfig and that value will always be
used. --enable-release implies a number of other changes. So if you just
want to restore the old build system behavior, set this variable in your
mozconfig.

Also, due to ongoing work around Rust integration in the build system, it
is dangerous to rely on manual `cargo` invocations to compile Rust because
bypassing the build system (not using `mach build`) may not use the same
set of RUSTFLAGS that direct `cargo` invocations do. Things were mostly in
sync before. But this change and anticipated future changes will cause more
drift. If you want the correct behavior, use `mach`.
___
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: Default Rust optimization level decreased from 2 to 1

[Emilio Cobos Álvarez]

On 4/25/18 6:11 PM, Gregory Szorc wrote:




On Apr 25, 2018, at 08:35, Emilio Cobos Álvarez  wrote:

There's a fair amount of people bitten by this constantly, which see long style 
profiling markers and what's really happening is that they're profiling a local 
opt build, and thus the Rust code in style has barely any optimization and is 
slow.

I know that shouldn't be a thing, and that people should --enable-release for 
profiling and all that. But given it happens, could we consider reverting this 
change?


I’m going to assert that the set of people who care about fast builds is 
greater than the set of people who care about profiling the style system on 
local builds. Do you disagree? Or is the slowness of the style system 
undermining the ability for the local build to be usable/useful in general?


This is not about people profiling the style system, I couldn't disagree 
with that.


This is about people doing general profiling, or profiling unrelated 
stuff, which see that the style system takes half of the time on their 
testcase, when in proper release builds it's a minor amount of the total 
time.



Regardless, I think the most productive conversation we can have is about how 
we can make the Firefox development UI guide people towards an optimal build 
configuration. The reality is that there are several “factions” of developers 
that each want different things from a build. There is no 
one-build-config-fits-all.

The build peers have long thought about adding the concept of “build profiles” 
to the build system. Think of them as in-tree mozconfigs for common, supported 
scenarios.

But without a forcing function making people choose an initial “profile,” 
people will continue to get bit by whatever default we choose.

We once printed a fatal message on first invocation of the build system. This 
was intended to be such a forcing function. But there was popular revolt and 
this feature was removed. Considering that I managed to upset a lot of people 
with this feature, I’m reluctant to go down that path again because it would 
seemingly undermine general support for the build system team and jeopardize 
our ability to make meaningful changes.




On 10/25/17 7:34 PM, Gregory Szorc wrote:
Compiling Rust code with optimizations is significantly slower than
compiling without optimizations. As was measured in bug 1411081, the
difference between rustc's -Copt-level 1 and 2 on an i7-6700K (4+4 cores)
for a recent revision of mozilla-central was 325s/802s wall/CPU versus
625s/1282s. This made Rust compilation during Firefox builds stand out as a
long pole and significantly slowed down builds.
Because we couldn't justify the benefits of level 2 for the build time
overhead it added, we've changed the build system default so Rust is
compiled with -Copt-level=1 (instead of 2).
Adding --enable-release to your mozconfig (the configuration for builds we
ship to users) enables -Copt-level=2. (i.e. we didn't change optimization
settings for builds we ship to users.)
Adding --disable-optimize sets to -Copt-level=0. (This behavior is
unchanged.)
If you want explicit control over -Copt-level, you can `export
RUSTC_OPT_LEVEL=` in your mozconfig and that value will always be
used. --enable-release implies a number of other changes. So if you just
want to restore the old build system behavior, set this variable in your
mozconfig.
Also, due to ongoing work around Rust integration in the build system, it
is dangerous to rely on manual `cargo` invocations to compile Rust because
bypassing the build system (not using `mach build`) may not use the same
set of RUSTFLAGS that direct `cargo` invocations do. Things were mostly in
sync before. But this change and anticipated future changes will cause more
drift. If you want the correct behavior, use `mach`.
___
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


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

Re: Intent to unprefix: ::-moz-selection.

[Emilio Cobos Álvarez]

Sorry for not following up on this earlier, Ehsan, somehow this slipped 
through my inbox.


On 4/6/18 1:26 AM, Ehsan Akhgari wrote:

::-moz-selection seems like a fairly popular feature.  PublicWWW claims
it's seen on a million sites:
https://publicwww.com/websites/%22%3A%3A-moz-selection%22/

After this change, is it feasible to detect the usage of the prefixed
pseudo-element so that existing code that uses ::-moz-selection would emit
a helpful console warning?  It would be nice to have a way to communicate
to web developers that they can now remove the Gecko specific rules.


It is feasible, but it's not clear to me how worth it is to land this 
now. In particular, we're going to keep parsing ::-moz-selection for now 
as an alias, since as you mentioned it's a fairly popular feature, and 
people won't be able to remove it for a while if they want to support 
ESR and similar.


If we decide to remove the prefixed version, developers would get a in 
the console automatically if they have the CSS error filter enabled that 
would look like:


  Unknown pseudo-class or pseudo-element ‘-moz-selection’.  Ruleset 
ignored due to bad selector.


Which should be enough to hint them that they can remove it.

 -- Emilio


On Mon, Mar 26, 2018 at 9:21 AM Emilio Cobos Álvarez 
wrote:


Hi,

In bug 509958 I intend to unprefix the ::-moz-selection pseudo-element.

The situation here is not great wrt the spec saying what we do, or what
other implementations do for that matter, see [1].

However other engines have shipped this unprefixed for a long time with
the same semantics that we implement, and we're seeing webcompat
problems due to people forgetting to include ::-moz-selection in their
style sheets, see bug 1427680 or bug 1448670 for some examples.

Given I don't think other engines have any incentive to implement what
the spec says (given it's slower, and has the chance of giving them
webcompat headache), and that we're compatible with them, I proposed to
adapt the spec to reality in that spec issue, and unprefix our
pseudo-element.

Let me know if there's any concerns with this plan. Thanks!

  -- Emilio

[1]: https://github.com/w3c/csswg-drafts/issues/2474
___
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: Intent to unprefix: ::-moz-selection.

[Emilio Cobos Álvarez]

In the CSSWG April F2F in Berlin this was extensively discussed and the 
CSSWG allowed / recommended us to unprefix this despite changing the spec.


Just landed the change unprefixing this on inbound, let me know if you 
see something broken because of it.


Thanks!

 -- Emilio

On 3/26/18 9:16 AM, Emilio Cobos Álvarez wrote:

Hi,

In bug 509958 I intend to unprefix the ::-moz-selection pseudo-element.

The situation here is not great wrt the spec saying what we do, or what
other implementations do for that matter, see [1].

However other engines have shipped this unprefixed for a long time with
the same semantics that we implement, and we're seeing webcompat
problems due to people forgetting to include ::-moz-selection in their
style sheets, see bug 1427680 or bug 1448670 for some examples.

Given I don't think other engines have any incentive to implement what
the spec says (given it's slower, and has the chance of giving them
webcompat headache), and that we're compatible with them, I proposed to
adapt the spec to reality in that spec issue, and unprefix our
pseudo-element.

Let me know if there's any concerns with this plan. Thanks!

  -- Emilio

[1]: https://github.com/w3c/csswg-drafts/issues/2474
___
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 unship: CSSStyleDeclaration.getPropertyCSSValue

[Emilio Cobos Álvarez]

Following up on this, no breakage whatsoever was reported, so I removed 
the pref and the API from the tree in bug 1408301.


Thanks,

 -- Emilio

On 3/23/18 7:23 PM, Emilio Cobos Álvarez wrote:

Hi,

Bug 1408301 tracks unshipping CSSStyleDeclaration.getPropertyCSSValue.

This is a non-standard API only implemented by Mozilla, and that
generally can be replaced by usage of the standard .getPropertyValue.

We added a use counter and deprecation warning in bug 474655. The data
seems to indicate the usage is low, so I'm trying to remove this API on
Nightly only to see if there's any breakage in bug 1448415, tentatively
under the pref layout.css.getPropertyCSSValue.enabled.

Let me know if there's any concern with doing this, and please file bugs
blocking either bug 1448415 or bug 1408301 if you see something broken
because of this.

Thanks!

  -- Emilio
___
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 unship: CSSStyleDeclaration.getPropertyCSSValue

[Emilio Cobos Álvarez]

Hmm, so my reasoning is that this had already been unshipped during 61 
Nightly so that gave us two nightlies + one beta cycle. But unshipping 
it in 61 entirely is probably better indeed.


Filed bug 1461092.

 -- Emilio

On 5/12/18 1:03 PM, Xidorn Quan wrote:

Given that you are unshipping this... probably we should uplift a pref switch 
to beta to make it unship in 61 directly, and then remove in 62. I think that's 
probably safer than not unshipping it in 61 but removing directly in 62.

- Xidorn

On Sat, May 12, 2018, at 7:30 PM, Emilio Cobos Álvarez wrote:

Following up on this, no breakage whatsoever was reported, so I removed
the pref and the API from the tree in bug 1408301.

Thanks,

   -- Emilio

On 3/23/18 7:23 PM, Emilio Cobos Álvarez wrote:

Hi,

Bug 1408301 tracks unshipping CSSStyleDeclaration.getPropertyCSSValue.

This is a non-standard API only implemented by Mozilla, and that
generally can be replaced by usage of the standard .getPropertyValue.

We added a use counter and deprecation warning in bug 474655. The data
seems to indicate the usage is low, so I'm trying to remove this API on
Nightly only to see if there's any breakage in bug 1448415, tentatively
under the pref layout.css.getPropertyCSSValue.enabled.

Let me know if there's any concern with doing this, and please file bugs
blocking either bug 1448415 or bug 1408301 if you see something broken
because of this.

Thanks!

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


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

Re: Intent to unship: CSSStyleDeclaration.getPropertyCSSValue

[Emilio Cobos Álvarez]

On 5/12/18 9:26 PM, L. David Baron wrote:

It seems risky to remove the code (and then have the risk of doing
things that depend on the code being gone) until we've actually
shipped the pref-flip that turns the API off to release.  (Or am I
misunderstanding, and that's already happened?)  Until we've safely
shipped it to release, we can't really be sure we won't have to
change our minds.


So, the code for the API isn't gone yet. Indeed getPropertyValue is 
still backed by what used to be getPropertyCSSValue (see 
nsComputedDOMStyle::GetPropertyCSSValueWithoutWarning).


I plan to change the getComputedStyle back-end in bug 1408300 to use the 
Rust code, and unexpose (and remove cycle-collection bits from) related 
interfaces in bug 1459871.


With that, I planned to keep the related code around for a bit until 
it's unshipped from release, then remove all the CSSValue related code 
and similar bits.


Does that seem like a reasonable path forward?

Thanks!

 -- Emilio


-David

On Saturday 2018-05-12 11:30 +0200, Emilio Cobos Álvarez wrote:

Following up on this, no breakage whatsoever was reported, so I removed the
pref and the API from the tree in bug 1408301.

Thanks,

  -- Emilio

On 3/23/18 7:23 PM, Emilio Cobos Álvarez wrote:

Hi,

Bug 1408301 tracks unshipping CSSStyleDeclaration.getPropertyCSSValue.

This is a non-standard API only implemented by Mozilla, and that
generally can be replaced by usage of the standard .getPropertyValue.

We added a use counter and deprecation warning in bug 474655. The data
seems to indicate the usage is low, so I'm trying to remove this API on
Nightly only to see if there's any breakage in bug 1448415, tentatively
under the pref layout.css.getPropertyCSSValue.enabled.

Let me know if there's any concern with doing this, and please file bugs
blocking either bug 1448415 or bug 1408301 if you see something broken
because of this.

Thanks!

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

Intent to unship: getPropertyCSSValue-related interfaces Rect, RGBColor, CSSValue, CSSPrimitiveValue and CSSValueList

[Emilio Cobos Álvarez]

Related to the unshipping of getPropertyCSSValue, in bug 1459871 I 
intend to unexpose the following related interfaces from the platform:


  Rect, RGBColor, CSSValue, CSSPrimitiveValue, CSSValueList

Instances of these objects could only be constructed via 
getPropertyCSSValue, so they're not useful in any way anymore.


Regarding the possibility of people relying on its mere presence to do 
some sort of browser detection, I'd say it's quite unlikely, given we're 
in this weird situation where WebKit implements and exposes them but 
Blink and Edge do not.


In any case, the native code backing the interfaces will remain in tree 
for a bit more, so we could always roll back if it happens to break 
something.


Let me know if there's any concern with proceeding.

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

PSA: No more mozilla::Move

[Emilio Cobos Álvarez]

Hi, just a quick PSA:

In bug 1465585 I switched all uses of mozilla::Move to std::move, and
removed the former.

The reasoning for that is that it allows compilers to detect misuses of
std::move and warn about them (-Wpessimizing-move / -Wself-move /
-Wreturn-std-move).

Beware of some local mac builds maybe being broken. That should be fixed
by bug 1270217 (thanks jwatt!).

Cheers,

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

Intent to Unship: -moz-window-opacity / -moz-window-transform / -moz-window-transform-origin CSS properties.

[Emilio Cobos Álvarez]

Hi,

In bug 1419695 I plan to unship the CSS properties mentioned in the 
subject of this email.


The only reason they were exposed were because they landed as 
accidentally exposed given the confusing semantics of the "internal" 
properties (that just hides them from CSSOM enumeration code), and the 
ones that are really chrome-only / ua-only.


We couldn't just hide them because the style system was refusing to 
transition chrome-only / ua-only properties, and the mac code does 
transition them.


In any case, these properties don't have any effect on content pages, 
since the properties are only used for Mac OS windows, so other than 
hiding them from the OM / forbidding them from parsing, this shouldn't 
have any other compat impact.


Let me know if you think otherwise. Thanks!

Thanks!

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

Re: Intent to Unship: -moz-window-opacity / -moz-window-transform / -moz-window-transform-origin CSS properties.

[Emilio Cobos Álvarez]

The subject of the email should probably say "from content pages" :)

On 6/2/18 5:18 PM, Emilio Cobos Álvarez wrote:

Hi,

In bug 1419695 I plan to unship the CSS properties mentioned in the 
subject of this email.


The only reason they were exposed were because they landed as 
accidentally exposed given the confusing semantics of the "internal" 
properties (that just hides them from CSSOM enumeration code), and the 
ones that are really chrome-only / ua-only.


We couldn't just hide them because the style system was refusing to 
transition chrome-only / ua-only properties, and the mac code does 
transition them.


In any case, these properties don't have any effect on content pages, 
since the properties are only used for Mac OS windows, so other than 
hiding them from the OM / forbidding them from parsing, this shouldn't 
have any other compat impact.


Let me know if you think otherwise. Thanks!

Thanks!

  -- Emilio
___
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 unship: XUL display values from content pages

[Emilio Cobos Álvarez]

Hi,

In bug 1288572 I plan to remove the XUL display property values from 
content pages.


This includes the following display values:

  -moz-grid
  -moz-inline-grid
  -moz-grid-group
  -moz-grid-line
  -moz-stack
  -moz-inline-stack
  -moz-deck
  -moz-popup
  -moz-groupbox

This was going to include also -moz-box and -moz-inline-box, but there's 
a chance that those are used in content pages (though I doubt it), so 
I've added telemetry for those, and those will be hidden in a followup bug.


This will be all behind the 
layout.css.xul-display-values.content.enabled pref, in case there are 
surprises.


Let me know if there's any concern with proceeding here. Thanks!

 -- Emilio

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

Re: Coding style: brace initialization syntax

[Emilio Cobos Álvarez]

On 06/05/2018 06:48 PM, Eric Rahm wrote:

Reading back through I think the consensus, at least for initializer lists
was:

1. Prefer parenthesis, ie:
, mBool(true)
2. If using braces, maintain the same spacing you would use with
parenthesis, ie:
, mStructWithoutCtor{42}

1. was pragmatic as this is what we already do, 2. was for consistency with
1.


I personally would prefer one space at each side when using braces:

 , mFoo { 0 }

But people seemed to think that for consistency we should use no spaces 
with braces as well... So if more people agree with that, oh well :)


Though I really find no-spaces-around-braces harder to read, not only in 
constructors but in general initializer lists. For example, I find:


  return { Foo::Bar, bar, baz };

easier to read than:

  return {Foo::Bar, bar, baz};

(and also more consistent with what you write in Javascript or Rust).

But nor sure if this discussion would expand to those cases.

 -- Emilio



To answer Bogdan's question, it looks like we prefer [1], although it would
be nice to see that codified in our style doc.

jya, you make some interesting points, but we should keep the scope of this
discussion focused. You might want to raise them in separate threads --
"Should we recommend initialization at member declaration", "Should we
recommend where a ctor should is defined", etc.

-e


On Tue, Jun 5, 2018 at 5:50 AM, Jean-Yves Avenard 
wrote:





On 5 Jun 2018, at 12:54 pm, bposteln...@mozilla.com wrote:

I would like to resurrect this thread since it would help us a lot for

bug 1453795 to come up to a consensus on when to use bracelets and when to
use parenthesis. Also I must point out a thing here, that was also
mentioned here earlier, that there are situations where we cannot use
parenthesis. This is when we want to initialize a structure that doesn't
have a ctor, like:

[1]
struct str {
  int a;
  int b;
};

class Str {
  str s;
  int a;
public:
  Str() : s{0}, a(0) {}
};

Also it would help a lot if we would establish how many, spaces should

be between the parenthesis or the bracelets, like how do we prefer [1] or
[2]


[2]
class Str {
  str s;
  int a;
public:
  Str() : s{ 0 }, a( 0 ) {}
};

I don't have a personal preference here, but right now there are several

places in our code that combine spaces between parenthesis/bracelets with
no spaces.

The current coding style: https://developer.mozilla.org/
en-US/docs/Mozilla/Developer_guide/Coding_Style states to not use space.

There’s no case where a parenthesis should be followed by a space.

Many things wrong here:
First the bracket { should be on a new line :

class/struct str
{
…
}

Initialization are to be on multiple-lines.

clang-format would have made it:
   class Str
   {
 str s;
 int a;

   public:
 Str()
   : s{ 0 }
   , a(0)
 {
 }
   };

IMHO, should be going for C++11 initializer, it’s much clearer, and avoid
duplicated code when you need multiple constructors.
What is str? I assume not a plain object, so it should have its own
initializer.

so it all becomes:
   class Str
   {
 str s;
 int a = 0;

   public:
 Str() {}
   };

or:
   class Str
   {
 str s;
 int a = 0;

   public:
 Str() = default;
   };

(and I prefer constructors to be defined at the start of the class
definition)

My $0.02
___
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: Intent to ship: shape-outside

[Emilio Cobos Álvarez]

On 06/08/2018 08:08 PM, Bradley Werth wrote:

On Monday June 11th, I intend to land Bug 1457297
 which will enable
shape-outside for all channels in Firefox 62. The feature has been on by
default in Nightly 61, and available in other channels behind the
layout.css.shape-outside.enabled pref. Blink and Webkit browsers already
implement this feature.


Not necessarily a concern, but do you know what's the state regarding 
interop?


I saw you upstreamed a lot of test-cases for the feature and edge cases 
while developing it to WPT, which is awesome. Do WebKit and Blink all 
pass those tests? If not, why? Are there bugs on file for those?


I know Blink has bugs automatically on file for new WPT failures like 
[1], but I think it's still worth to describe why they fail it if we 
know why.


Thanks for working on this!

[1]: https://bugs.chromium.org/p/chromium/issues/detail?id=851292

 -- Emilio


There are some pending issues in Bug 1466231
 and Bug 1466231
 that should either
be in before 62 merge date or safely uplift-able.

Let me know if there are any concerns, thanks,
Brad
___
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 ship: shape-outside

[Emilio Cobos Álvarez]

On 6/11/18 10:19 AM, Bradley Werth wrote:

On Mon, Jun 11, 2018 at 7:54 AM, Emilio Cobos Álvarez 
wrote:


On 06/08/2018 08:08 PM, Bradley Werth wrote:


On Monday June 11th, I intend to land Bug 1457297
<https://bugzilla.mozilla.org/show_bug.cgi?id=1457297> which will enable
shape-outside for all channels in Firefox 62. The feature has been on by
default in Nightly 61, and available in other channels behind the
layout.css.shape-outside.enabled pref. Blink and Webkit browsers already
implement this feature.



Not necessarily a concern, but do you know what's the state regarding
interop?

I saw you upstreamed a lot of test-cases for the feature and edge cases
while developing it to WPT, which is awesome. Do WebKit and Blink all pass
those tests? If not, why? Are there bugs on file for those?



Some of the WPT I added show interop problems in both WebKit and Blink.
Tests for Bug 1464113 <https://bugzilla.mozilla.org/show_bug.cgi?id=1464113>
and Bug 1460041 <https://bugzilla.mozilla.org/show_bug.cgi?id=1460041>
indicate that other UAs have trouble with shapes defined with negative
offsets relative to their containing blocks. I haven't filed bugs -- I'm
not familiar with the process for those repositories -- and I don't know
whether bugs have been filed.


The relevant URLs are crbug.com/new and bugs.webkit.org. It would be 
useful for us to get feedback from them, and for them to get it triaged 
and diagnosed faster. I don't think it's needed for enabling the pref, 
but it'd certainly be nice.


Thanks again for all your work on getting this ready for shipping, and 
for Ting-Yu for all the previous work! :)


 -- Emilio


___
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 Unship: -moz-is-resource-document media feature from content pages.

[Emilio Cobos Álvarez]

Hi,

In bug 1468854 I plan to unship the -moz-is-resource-document media 
feature from content pages.


This was introduced to prevent leaking the theme from SVG resource 
documents in bug 686581.


I expect no compat fallout whatsoever since it can only match, well, in 
SVG resource documents, and it was meant to be an internal media feature 
in the first place.


Let me know if there's any concern about this.

Thanks,

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

Slightly delayed Intent to Ship: getComputedStyle changes on some edge cases.

[Emilio Cobos Álvarez]

Hi,

Just something I figure was worth sending an email for, due to the 
potential (ideally positive) web-compat impact.


In bug 1467722[1], I brought us closer to the spec and to WebKit / Blink 
in terms of what happens when we can't return a style for an element 
from getComputedStyle (mostly relevant for hidden iframes, for example).


Before that change, we either returned null (if you called 
getComputedStyle _after_ hiding the iframe) or threw an 
NS_ERROR_NOT_AVAILABLE exception (if you call getComputedStyle before 
hiding the frame, but getPropertyValue afterwards).


Now we'll never return null for getComputedStyle(..), and we'll return 
an empty string and a .length of zero when we can't get a style, 
aligning us with other browser's 'no style' behavior[2].


The idea is that this will prevent most of the pain that bugs like [3] 
cause us, and be a step into aligning with the CSSOM spec more closely.


This landed on time for 62, and I don't expect any breakage from it, but 
let me know if you see something.


 -- Emilio

[1]: https://bugzilla.mozilla.org/show_bug.cgi?id=1467722
[2]: WebKit and Blink do not return a style for an element outside of 
the document, for example, and they behave that way in that case.

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

Intent to ship: Some of the mediaqueries-4 syntax improvements.

[Emilio Cobos Álvarez]

Hi,

In bug 145 I plan to land most of the syntax improvements to 
mediaqueries-4.


Some of the features included are:

 * Allowing operators such as >, <, >=, or <= in media feature 
expressions, which allows to properly exclude media queries in a way 
min-* and max-* cannot, like:


 @media (width >= 900px) { some rules }
 @media (width < 900px) { some other rules }

 Guarantees that either `some rules` or `some other rules` apply, which 
is something that is not guaranteed by the existing syntax (see [1] or 
[2], for example).


 * Or expressions, and arbitrary expression nesting like:

 @media ((width >= 500px) and (width <= 900px)) or (not 
(orientation: portrait))


Things that are _not_ included are:

 * The range syntax, or allowing values before the feature name, that is:

@media (500px > width) or (500px < width < 900px)

   This is nice, but not so trivial to implement, and you can either 
reverse the expression (`(width <= 500px)` in the first case), or use 
the expanded version of it using `and` expressions for the second.


 * The changes to serialization and parsing that allows basically 
anything in a feature expression to be valid, that is, treating as a 
valid media query something like:


@media (orientation: portrait) or (garbage)

Bug 1469174 and bug 1469173 are tracking those two, respectively.

Let me know if you find unknown issues, or think we shouldn't ship this.

Thanks!

 -- Emilio

[1]: 
http://damienclarke.me/code/posts/those-1px-gaps-between-media-queries-can-be-a-problem

[2]: https://github.com/twbs/bootstrap/issues/19197

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

Coding style: Making the `e` prefix for enum variants not mandatory?

[Emilio Cobos Álvarez]

Our coding style states that we should use an `e` prefix for enum 
variants, that is:


  enum class Foo { eBar, eBaz };

We're not really consistent about it: looking at layout/, we mostly use 
CamelCase, though we do have some prefixed enums. Looking at other 
modules, enum classes almost never use it either. DOM bindings also 
don't use that prefix.


I think that with enum classes the usefulness of the prefix is less 
justified. Plus removing them would allow us to match the Rust coding 
style as well, which is nice IMO.


Would anybody object to making the prefix non-mandatory, removing that 
line from the coding style doc? Maybe only making it non-mandatory for 
enum classes?


Thanks,

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

Re: Coding style: Making the `e` prefix for enum variants not mandatory?

[Emilio Cobos Álvarez]

And Kris pointed out that we already had another huge thread on this:


https://groups.google.com/d/msg/mozilla.dev.platform/WAuySoTfq_w/-DggRotpBQAJ

Looks like there wasn't agreement on that one... But oh well, don't want 
to repeat a lot of that discussion.


I think the argument for consistency with the other systems language we 
have in-tree, the fact that it's not predominant (at least for enum 
classes) even though it is in the coding style, and that there wasn't 
agreement in the previous thread are good reasons for not enforcing it, 
but...


 -- Emilio

On 6/25/18 10:41 PM, Emilio Cobos Álvarez wrote:
Our coding style states that we should use an `e` prefix for enum 
variants, that is:


   enum class Foo { eBar, eBaz };

We're not really consistent about it: looking at layout/, we mostly use 
CamelCase, though we do have some prefixed enums. Looking at other 
modules, enum classes almost never use it either. DOM bindings also 
don't use that prefix.


I think that with enum classes the usefulness of the prefix is less 
justified. Plus removing them would allow us to match the Rust coding 
style as well, which is nice IMO.


Would anybody object to making the prefix non-mandatory, removing that 
line from the coding style doc? Maybe only making it non-mandatory for 
enum classes?


Thanks,

  -- Emilio
___
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 ship: Some of the mediaqueries-4 syntax improvements.

[Emilio Cobos Álvarez]

On 6/25/18 11:01 PM, L. David Baron wrote:

How does the particular set of features that you're planning to ship
vs. not ship align with what other browsers have shipped (or are
close shipping)?


I'm not aware of any other browser implementing or shipping any of the 
changes from MQ3 to MQ4, so we'd be the first to support these.


This subset is somewhat straight-forward, and spec author feedback is 
clear I think:


  https://lists.w3.org/Archives/Public/www-style/2017Feb/0036.html

I'd be fine not shipping it for now and keeping it behind a pref, but I 
don't think it's worth it given how unlikely it is to change. Let me 
know if you think otherwise though.


 -- Emilio



-David

On Monday 2018-06-25 21:13 +0200, Emilio Cobos Álvarez wrote:

In bug 145 I plan to land most of the syntax improvements to
mediaqueries-4.

Some of the features included are:

  * Allowing operators such as >, <, >=, or <= in media feature expressions,
which allows to properly exclude media queries in a way min-* and max-*
cannot, like:

  @media (width >= 900px) { some rules }
  @media (width < 900px) { some other rules }

  Guarantees that either `some rules` or `some other rules` apply, which is
something that is not guaranteed by the existing syntax (see [1] or [2], for
example).

  * Or expressions, and arbitrary expression nesting like:

  @media ((width >= 500px) and (width <= 900px)) or (not (orientation:
portrait))

Things that are _not_ included are:

  * The range syntax, or allowing values before the feature name, that is:

 @media (500px > width) or (500px < width < 900px)

This is nice, but not so trivial to implement, and you can either reverse
the expression (`(width <= 500px)` in the first case), or use the expanded
version of it using `and` expressions for the second.

  * The changes to serialization and parsing that allows basically anything
in a feature expression to be valid, that is, treating as a valid media
query something like:

 @media (orientation: portrait) or (garbage)

Bug 1469174 and bug 1469173 are tracking those two, respectively.

Let me know if you find unknown issues, or think we shouldn't ship this.



___
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 ship: inset-* logical properties.

[Emilio Cobos Álvarez]

Hi,

In bug 1464782 I renamed the long-shipped offset-* logical properties 
(offset-inline-start, offset-inline-end, offset-block-start, 
offset-block-end), which map to top / bottom / left / right, to their 
standard name (inset-*), and turned the later into aliases.


No other browser implements this so far (nor the offset-*), but Blink is 
working on logical properties support and presumably they'll only 
support the inset-* ones.


There's WPT tests for this already, but they fail on Firefox because of 
bug 1116638, which is on my radar.


Let me know if there's any concern with such thing. As part of it I plan 
to deprecate and remove the offset-* properties, but I'll send a 
separate intent for that in a few :)


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

Intent to deprecate and remove: offset-* logical properties.

[Emilio Cobos Álvarez]

In bug 1464782 I renamed these to inset-*, and left the offset-* 
properties as an alias behind a (enabled-by-default) pref:


  layout.css.offset-logical-properties.enabled

I want it to turn it off by default sooner rather than later, given no 
other engine supports it, but keep the pref around for at least a 
release, following David's advice in [1].


I just filed bug 1471838 for that.

There's (somewhat unlikely) chance of compat fallout if some page relies 
on these, but my understanding is that this is somewhat unlikely given 
no other engine supports them yet, and we're at the beginning of a cycle 
so toggling the pref back on should be trivial should we need to do so.


 -- Emilio

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

Re: Coding style: Making the `e` prefix for enum variants not mandatory?

[Emilio Cobos Álvarez]

I asked kats@ (which added the list item regarding enums) and he was 
fine removing it from the coding style, so given that and the rest of 
the thread I've edited the page accordingly.


Cheers,

 -- Emilio

On 6/26/18 2:46 AM, Jeff Gilbert wrote:

If we can't agree, it shouldn't be in the style guide.

On Mon, Jun 25, 2018 at 2:17 PM, Peter Saint-Andre  wrote:

And perhaps good reason for removing it from the style guide? ;-)

On 6/25/18 3:08 PM, Emilio Cobos Álvarez wrote:

And Kris pointed out that we already had another huge thread on this:


https://groups.google.com/d/msg/mozilla.dev.platform/WAuySoTfq_w/-DggRotpBQAJ


Looks like there wasn't agreement on that one... But oh well, don't want
to repeat a lot of that discussion.

I think the argument for consistency with the other systems language we
have in-tree, the fact that it's not predominant (at least for enum
classes) even though it is in the coding style, and that there wasn't
agreement in the previous thread are good reasons for not enforcing it,
but...

  -- Emilio

On 6/25/18 10:41 PM, Emilio Cobos Álvarez wrote:

Our coding style states that we should use an `e` prefix for enum
variants, that is:

enum class Foo { eBar, eBaz };

We're not really consistent about it: looking at layout/, we mostly
use CamelCase, though we do have some prefixed enums. Looking at other
modules, enum classes almost never use it either. DOM bindings also
don't use that prefix.

I think that with enum classes the usefulness of the prefix is less
justified. Plus removing them would allow us to match the Rust coding
style as well, which is nice IMO.

Would anybody object to making the prefix non-mandatory, removing that
line from the coding style doc? Maybe only making it non-mandatory for
enum classes?

Thanks,

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

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

Re: Coding style: Making the `e` prefix for enum variants not mandatory?

[Emilio Cobos Álvarez]

On 6/29/18 12:15 AM, Mike Hommey wrote:

On Thu, Jun 28, 2018 at 05:27:23PM +0200, Emilio Cobos Álvarez wrote:

I asked kats@ (which added the list item regarding enums) and he was fine
removing it from the coding style, so given that and the rest of the thread
I've edited the page accordingly.


Not that I'd oppose, but I'll note that neither kats, nor participants to
this thread so far are peers of the "C++/Rust usage, tools, and style"
module.


Oh, I didn't realize that those peers were the only ones to be able to 
update the style guide, sorry. I guess it makes sense.


I can revert the change if needed and try to get sign-off from some of 
those.


Apologies again, I just followed the procedure that was followed in the 
previous thread to add the rule. Let me know if you want that change 
reverted and I'll happily do so.


 -- Emilio



Mike


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

Intent to Ship: Logical properties in computed style objects.

[Emilio Cobos Álvarez]

Hi,

In bug 1116638 I made changes so that getPropertyValue in computed style 
objects would correctly expose logical properties.


This is more of a bug-fix than a new feature, but it was probably worth 
sending an email to the list.


Let me know if there's any concern with this.

Thanks!

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

Re: Coding style: Making the `e` prefix for enum variants not mandatory?

[Emilio Cobos Álvarez]

On 7/6/18 8:21 PM, Ehsan Akhgari wrote:

On Fri, Jun 29, 2018 at 2:36 PM, smaug  wrote:


On 06/29/2018 05:58 PM, Boris Zbarsky wrote:


On 6/29/18 10:30 AM, Nathan Froyd wrote:


Given the language-required qualification for
`enum class` and a more Rust-alike syntax, I would feel comfortable
with saying CamelCase `enum class` is the way to go.



For what it's worth, I agree.  The point of the "e" prefix is to
highlight that you have an enumeration and add some poor-man's namespacing
for a potentially large number of common-looking names, and the
language-required qualification already handles both of those.



That works if we're consistently naming static variables and such using
s-prefix etc, so that
class Foo1
{
   static int sBar;
};
Foo1::sBar

is clearly different to
enum class Foo2
{
   Bar
};

Foo2::Bar


(personally I'd still prefer using e-prefix always with enums, class or
not. Foo1::sBar vs Foo2::eBar is easier to distinguish than Foo1::sBar vs
Foo2::Bar)



Looking at other popular style guidelines, the Google C++ style requires
the 'k' prefix on both enum and enum class <
https://google.github.io/styleguide/cppguide.html#Enumerator_Names>
presumably because it doesn't require any special prefix for static
members.  But given that ours does, it seems that in the case of enum
classes, not using a prefix should be acceptable.

Based on Nathan's analysis (thanks, Nathan!) and the previous thread, it
seems like Emilio's edit should be reverted and a note should be added
about the usage of CamelCase for enum classes.  Emilio, do you mind doing
that, please?


Sure, just did:


https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style$compare?locale=en-US&to=1394287&from=1392024

Feel free to clarify or change as you think it's more useful.

Thanks Ehsan!

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

Re: Fission MemShrink Newsletter #1: What (it is) and Why (it matters to you)

[Emilio Cobos Álvarez]

Thanks for doing this!

Just curious, is there a bug on file to measure excess capacity on 
nsTArrays and hash tables?


WebKit has a bunch of bugs like:

  https://bugs.webkit.org/show_bug.cgi?id=186709

Which seem relevant.

 -- Emilio

On 07/10/2018 08:19 PM, Kris Maglione wrote:

Welcome to the first edition of the Fission MemShrink newsletter.[1]

In this edition, I'll sum up what the project is, and why it matters to 
you. In subsequent editions, I'll give updates on progress that we've 
made, and areas that we'll need to focus on next.[2]



The Fission MemShrink project is one of the most easily overlooked 
aspects of Project Fission (also known as Site Isolation), but is 
absolutely critical to its success. And will require a company- and 
community-wide effort effort to meet its goals.


The problem is thus: In order for site isolation to work, we need to be 
able to run *at least* 100 content processes in an average Firefox 
session. Each of those processes has its own base memory overhead—memory 
we use just for creating the process, regardless of what's running in 
it. In the post-Fission world, that overhead needs to be less than 10MB 
per process in order to keep the extra overhead from Fission below 1GB. 
Right now, on our best-cast platform, Windows 10, is somewhere between 
17 and 21MB. Linux and OS-X hover between 25 and 35MB. In other words, 
between 2 and 3.5GB for an ordinary session.


That means that, in the best case, we need to reduce the memory we use 
in content processes by *at least* 7MB. The problem, of course, is that 
there are only so many places we can cut memory without losing 
functionality, and even fewer places where we can make big wins. But, 
there are lots of places we can make small and medium-sized wins.


So, to put the task into perspective, of all of the places we can cut a 
certain amount of overhead, here are the number of each that we need to 
fix in order to reach 1MB:


250KB:   4
100KB:  10
75KB:   13
50KB:   20
20KB:   50
10KB:  100
5KB:   200

Now remember: we need to do *all* of these in order to reach our goal. 
It's not a matter of one 250KB improvement or 50 5KB improvements. It's 
4 250KB *and* 200 5KB improvements. There just aren't enough places we 
can cut 250KB. If we fall short in any of those areas, Project Fission 
will fail, and Firefox will be the only major browser without site 
isolation.


But it won't fail, because all of you are awesome, and this is a totally 
achievable goal if we all throw our effort behind it.


Essentially what this means, though, is that if we identify an area of 
overhead that's 50KB[3] or larger that can be eliminated, it *has* to be 
eliminated. There just aren't that many large chunks to remove. They all 
need to go. And if an area of code has a dozen 5KB chunks that can be 
eliminated, maybe they don't all have to go, but at least half of them 
do. The more the better.



To help us triage these issues, we have a tracking bug 
(https://bugzil.la/memshrink-content), and a per-bug whiteboard tag 
([overhead:...]) which gives an estimate of how much per-process 
overhead we believe fixing that bug would eliminate. Please feel free to 
add blockers to the tracking bug if you think they're relevant, and to 
add or update [overhead] tags if you have reasonable estimates.



With all of that said, here's a brief update of the progress we've made 
so far:


In the past month, unique memory per process[4] has dropped 3-4MB[5], 
and JS memory usage in particular has dropped 1.1-1.9MB.


Particular credit goes to:

* Eric Rahm added an AWSY test suite to track base content process memory
   (https://bugzil.la/1442361). Results:

    Resident unique: 
https://treeherder.mozilla.org/perf.html#/graphs?series=mozilla-central,1684862,1,4&series=mozilla-central,1684846,1,4&series=mozilla-central,1685133,1,4&series=mozilla-central,1685127,1,4 

    Explicit allocations: 
https://treeherder.mozilla.org/perf.html#/graphs?series=mozilla-inbound,1706218,1,4&series=mozilla-inbound,1706220,1,4&series=mozilla-inbound,1706216,1,4 

    JS: 
https://treeherder.mozilla.org/perf.html#/graphs?series=mozilla-central,1684866,1,4&series=mozilla-central,1685137,1,4&series=mozilla-central,1685131,1,4 



* Andrew McCreight created a tool for tracking JS memory usage, and 
figuring

   out which scripts and objects are responsible for how much of it
   (https://bugzil.la/1463569).

* Andrew and Nika Layzell also completely rewrote the way we handle 
XPIDL type
   info so that it's statically compiled into the executable and shared 
between

   all processes (https://bugzil.la/1438688, https://bugzil.la/1444745).

* Felipe Gomes split a bunch of code out of frame scripts so that it 
could be
   lazily loaded only when needed (https://bugzil.la/1467278, ...) and 
added a
   whitelist of JSMs that are allowed to be loaded at content process 
startup

   (https://bugzil.la/1471066)

* I did a bit of this too, and also prevented us from loading s

Intent to unship: display: -moz-box and display: -moz-inline-box from content pages.

[Emilio Cobos Álvarez]

Hi,

In bug 1477553 I intend to disable the ability for content to specify 
display: -moz-box and -moz-inline-box, which will be consistent with 
what we did for the rest of -moz- prefixed values in bug 1288572.


We have a use counter for this in [1], which is somewhat high. This 
could however be a bit misleading, since people tend to confuse these 
values (XUL flexbox) with the equivalent -webkit-prefixed ones (legacy 
HTML flexbox). This confusion caused enough issues for us that we had to 
add a hack in the style engine to ignore -moz- prefixed values when the 
-webkit- prefixed value is specified before-hand [2], see bug 1407701 & 
friends.


Given we don't / can't collect URLs from pages where the use counter is 
hit, we can't asses whether the pages where it's hit are using it 
intentionally or just by adding prefixes. Though my gut feeling is that 
it's mostly the later...


In any case, given that, for now I plan to just hide them on Nightly and 
early beta, before doing it for the release channel in a couple releases 
if everything goes well and there's no reported breakage.


This will all be behind the pref:

  layout.css.xul-box-display-values.content.enabled

Let me know if there's any concern with this, and please file bugs 
blocking that one if you find broken stuff.


Thanks!

 -- Emilio

[1]: 
https://georgf.github.io/usecounters/index.html#kind=page&group=DEPRECATED&channel=beta&version=62
[2]: 
https://searchfox.org/mozilla-central/rev/ad36eff63e208b37bc9441b91b7cea7291d82890/servo/components/style/properties/declaration_block.rs#519

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