Re: [Wikitech-l] Declaring methods final in classes

[Aryeh Gregor]

On Fri, Aug 30, 2019 at 10:09 PM Krinkle  wrote:
> For anything else, it doesn't really work in my experience because PHPUnit
> won't actually provide a valid implementation of the interface. It returns
> null for anything, which is usually not a valid implementation of the
> contract the class under test depends on. As a result, you end up with
> hundreds of lines of phpunit-mockery to basically reimplement the
> dependency's general functionality from scratch, every time.

In my experience, classes that use a particular interface usually call
only one or two methods from it, which can usually be replaced by
returning fixed values or at most two or three lines of code. The
implementations of the stub methods usually can be very simple,
because you know exactly the context that they'll be called and you
can hard-code the results. It could sometimes not be worth it to mock
for the reason you give, but in my experience that's very much the
exception rather than the rule.

> I have seen
> this too many times and fail to see the added value for all that complexity
> and maintenance, compared to simply letting the actual class do its work.
> Practically speaking, what kind of problem would that test methodology
> catch? I believe others may have a valid answer to this, but I haven't yet
> seen it. Perhaps it's a source of problems I tend to prevent by other
> means. There's more than one way to catch most problems :)

Consider a class like ReadOnlyMode. It has an ILoadBalancer injected.
The only thing it uses it for is getReadOnlyReason(). When testing
ReadOnlyMode, we want to test "What happens if the load balancer
returns true? What about false?" A mock allows us to inject the
required info with a few lines of code. If you use a real load
balancer, you're creating a number of problems:

1) Where do you get the load balancer from? You probably don't want to
manually supply all the parameters. If you use MediaWikiServices,
you're dependent on global state, so other tests can interfere with
yours. (Unless you tear down the services for every test, and see next
point.)

2) It's a waste of resources to run so much code just to generate
"true" or "false". One reason unit tests are so much faster than
integration tests is because they don't have to do all this setup of
the services.

3) I want an ILoadBalancer whose getReadOnlyReason() will return true
or false, so I can test how ReadOnlyMode will behave. How do I get a
real one to do that? There's no setReadOnlyReason() in ILoadBalancer.
Now I have to explore a concrete implementation and its dependencies
to figure out how to get it to return "true" or "false". Then anyone
reviewing my code has to do the same to verify that it's correct. This
is instead of a few lines of code that are contained within my test
file itself and are checkable for correctness via cursory inspection.

4) If the contract or implementation details of this totally separate
interface/class change, it will break my test. Now anyone who changes
LoadBalancer's implementation details risks breaking tests of classes
that depend on it. Worse, they might make the tests incorrect but
still pass. Perhaps I was setting readOnlyReason to true somehow, and
now that way no longer works, so it's really returning false -- but
maybe for some other reason my test now passes (perhaps other changes
made at the same time). Probably nobody will ever notice until an
actual bug arises.

5) In some cases you want to know that a certain method is being
called with certain parameters, but it's nontrivial to check
indirectly. For instance, suppose a certain method call is supposed to
update a row in the database. Even if you didn't mind giving it a real
database despite all the above reasons, how are you going to test the
correct row is being updated? You'd have to populate an initial row
(make sure the database gets reset afterwards!) and check that it was
updated properly. But what if a refactor introduced a bug in the WHERE
clause that happens not to stop the query from working in your case
but might cause problems on production data? Maybe the bug changed the
WHERE clause to one that happens to select your row of test data
despite being wrong. Trying to test every possible set of preexisting
data is not feasible. What's extremely simple is just testing that the
expected query is issued to a mock.

6) Performance is important to us. The direct way to test performance
is by measuring test run time, but this is unreliable because it's
heavily affected by load on the test servers. Also, lots of
performance issues only come up on large data sets, and we're probably
not going to run tests routinely on databases the size of Wikipedia's.
One way to help catch inadvertent performance regressions is to test
that the means of ensuring performance are being used properly. For
instance, if a method is supposed to first check a cache and only fall
back to the database for a cache miss, we want to test that the
database query 

Re: [Wikitech-l] Declaring methods final in classes

[Aryeh Gregor]

On Thu, Aug 29, 2019 at 5:30 PM Daniel Kinzler  wrote:
> But subclassing across module boundaries should be restricted to classes
> explicitly documented to act as extension points. If we could enforce this
> automatically, that would be excellent.

Well, for classes that aren't supposed to be subclassed at all, we
could mark them final. :)

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] Declaring methods final in classes

[Aryeh Gregor]

On Thu, Aug 29, 2019 at 1:02 AM Krinkle  wrote:
> What did you want to assert in this test?

In a proper unit test, I want to completely replace all non-value
classes with mocks, so that they don't call the actual class' code.
This way I can test the class under test without making assumptions
about other classes' behavior.

This is not possible at all if any method is declared final. As soon
as the class under test calls a final method, you cannot mock the
object. This is without any use of expects() or with() -- even just
method()->willReturn().

> I find there is sometimes a tendency for a test to needlessly duplicate the
> source code by being too strict on expecting exactly which method is called
> to the point that it becomes nothing but a more verbose version of the
> source code; always requiring a change to both.
>
> Personally, I prefer a style of testing where it providers a simpler view
> of the code. More like a manual of how it should be used, and what
> observable outcome it should produce.

The idea of good unit tests is to allow refactoring without having to
worry too much that you're accidentally changing observable behavior
in an undesired way. Ideally, then, any observable behavior should be
tested. Changes in source code that don't affect observable behavior
will never necessitate a change to a test, as long as the test doesn't
cheat with TestingAccessWrapper or such.

This includes tests for corner cases where the original behavior was
never considered or intended. This is obviously less important to test
than basic functionality, but in practice, callers often accidentally
depend on all sorts of incidental implementation details. Thus
ideally, they should be tested. If the test needs to be updated, that
means that some caller somewhere might break, and that should be taken
into consideration.

On Thu, Aug 29, 2019 at 1:12 AM Aaron Schulz  wrote:
> Interfaces will not work well for protected methods that need
> to be overriden and called by an abstract base class.

If you have an interface that the class implements, then it's possible
to mock the interface instead of the class, and the final method
problem goes away. Of course, your "final" is then not very useful if
someone implements the interface instead of extending the class, but
that shouldn't happen if your base class has a lot of code that
subclasses need.

On Thu, Aug 29, 2019 at 10:37 AM Daniel Kinzler  wrote:
> Narrow interfaces help with that. If we had for instance a cache interface 
> that
> defined just the get() and set() methods, and that's all the code needs, then 
> we
> can just provide a mock for that interface, and we wouldn't have to worry 
> about
> WANObjectCache or its final methods at all.

Any interface would solve the problem, even if it was just a copy of
all the public methods of WANObjectCache. That would be inelegant, but
another possible solution if we want to keep using final.

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] Declaring methods final in classes

[Aryeh Gregor]

On Wed, Aug 28, 2019 at 7:24 PM Lucas Werkmeister
 wrote:
> As far as I can tell, it actually strips final tokens from *any* PHP file
> that’s read, including by application code.

Yes, but only if you turn it on, and we'd only turn it on for tests.

> It seems to override the
> standard PHP handler for the file:// protocol, and rely on the fact that
> the PHP engine also uses that handler to read source code files.

I wonder how it interacts with an opcode cache. Is the cache going to
return the cached result based on mtime or whatever, meaning you'll
get a random mix of code with and without final and tests might fail
because they got a cached version of the file that wasn't
de-finalized? Or does it somehow know? (I don't see how it would.)

I filed an issue on this:
https://github.com/dg/bypass-finals/issues/12 Assuming it somehow
works with an opcode cache, it shouldn't have to be a huge performance
impact, because the files shouldn't be parsed often.

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] Declaring methods final in classes

[Aryeh Gregor]

On Tue, Aug 27, 2019 at 11:53 PM Daimona  wrote:
> Personally, I don't like these limitations in PHPUnit and the like. IMHO,
> they should never be a reason for changing good code.

I don't like these limitations either, but testing is an integral part
of development, and we need to code in a way that facilitates testing.
In each case we need to make a cost-benefit analysis about what's best
for the project. The question is whether there's any benefit to using
final that outweighs the cost to testability.

> And sometimes, methods have to be final.

Why do methods ever "have" to be final? Someone who installs an
extension accepts that they get whatever behavior changes the
extension makes. If the extension does something we don't want it to,
it will either work or not, but that's the extension's problem.

This is exactly the question: why do we ever want methods to be final?
Is there actually any benefit that outweighs the problems for testing?

> Anyway, some time ago I came across [1], which allows mocking final methods
> and classes. IIRC, it does that by removing the `final` keywords from the
> tokenized PHP code. I don't know how well it works, nor if it could degrade
> performance, but if it doesn't we could bring it in via composer.

That would be a nice solution if it works well. If someone wants to
volunteer to try to get it working, then we won't need to have this
discussion. But until someone does, the question remains.

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

[Wikitech-l] Declaring methods final in classes

[Aryeh Gregor]

I see that in some classes, like WANObjectCache, most methods are declared
final. Why is this? Is it an attempt to optimize?

The problem is that PHPUnit mocks can't touch final methods. Any ->method()
calls that try to do anything to them silently do nothing. This makes
writing tests harder.

If we really want these methods to be marked final for some reason, the
workaround for PHP is to make an interface that has all the desired
methods, have the class implement the interface, and make type hints all
refer to the interface instead of the class. But if there's no good reason
to declare the methods final to begin with, it's simplest to just drop it.
___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

[Bug 1823732] [NEW] jsonlint binary should be named "jsonlint", not "jsonlint-php"

[Aryeh Gregor]

Public bug reported:

AFAICT the program has no connection to PHP, beyond being written in
PHP, and there doesn't seem to be another command with a conflicting
name. Or at least, typing "jsonlint" at the command line doesn't suggest
installing any other packages. Debian seems to have changed the name to
"jsonlint-php" for some reason. The upstream name is "jsonlint":
https://github.com/Seldaek/jsonlint/tree/master/bin

** Affects: jsonlint (Ubuntu)
 Importance: Undecided
 Status: New

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/1823732

Title:
  jsonlint binary should be named "jsonlint", not "jsonlint-php"

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/jsonlint/+bug/1823732/+subscriptions

-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Re: [Wikitech-l] Extension test failures on PHP 7.1

[Aryeh Gregor]

On Mon, Oct 8, 2018 at 9:04 AM Kunal Mehta  wrote:
> In preparation for Wikimedia production switching to PHP 7.2, we need
> to get CI running using 7.2 (and for the rest of the MediaWiki world
> too!). But before we can do that, we'll need 7.1 to be passing first.

Did you mean 7.1 here instead of 7.2?  If not, for the record, I had a
test at one point that was passing on 7.0 and 7.2 but not 7.1.  So
this doesn't strictly speaking follow.  :)

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] ForkableMaintenance: thoughts and help with tests/code coverage

[Aryeh Gregor]

On Sun, Aug 12, 2018 at 4:48 AM, Brion Vibber  wrote:
> While working on some maintenance scripts for TimedMediaHandler I've been
> trying to make it easier to do scripts that use multiple parallel processes
> to run through a large input set faster.
>
> My proposal is a ForkableMaintenance class, with an underlying
> QueueingForkController which is a refactoring of the
> OrderedStreamingForkController used by (at least) some CirrusSearch
> maintenance scripts.

For what it's worth, when I saw ForkableMaintenance I thought of
forking an open-source project, not Unix fork().  Something like
ParallelMaintenance or ParallelizableMaintenance would better suggest
the desired meaning for me.

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] My Phabricator account has been disabled

[Aryeh Gregor]

On Thu, Aug 9, 2018 at 2:13 AM, MZMcBride  wrote:
> Are you sure about that? I think the Code of Conduct Committee _is_
> arguing that it's the use of the word "fuck" that was problematic here. If
> I had written "Why did you do that?!" instead of "What the fuck.", do you
> think I would have had my Phabricator account disabled for a week?
>
> As Alex asks on this mailing list: is using the abbreviated "wtf" form now
> considered a formal offense in tasks and commits? I genuinely do not know.

The main problem here that needs to be solved is communicating what
the problem was in a manner that is clear to the parties whom the CoC
committee seeks to deter.  A one-week ban is not going to help
anything if the object of the ban doesn't understand what about his
behavior elicited the ban.

From my experience in this type of thing, some people don't understand
what is meant by non-constructive forms of communication, and don't
know what types of statements will cause the person they're speaking
to to be upset and angry, nor how to rephrase them in a constructive
fashion.  This is something that takes quite a lot of practice, and
that fact might not be apparent to those who are naturally more
sensitive.  It's also something that comes naturally to someone who's
in a good mood and favorably disposed to the one they're speaking to,
and can be very difficult for the same person when he's angry.

Perhaps a member of the CoC committee should go over the scenario with
MZMcBride and discuss with him what alternative ways he should have
taken to address the problem, and what exactly the problem was with
how he did it.

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] My Phabricator account has been disabled

[Aryeh Gregor]

On Wed, Aug 8, 2018 at 11:29 PM, Amir Ladsgroup  wrote:
> 3) not being able to discuss cases clearly also bothers me too as I can't
> clarify points. But these secrecy is there for a reason. We have cases of
> sexual harassment in Wikimedia events, do you want us to communicate those
> too? And if not, where and who supposed to draw the line between public and
> non-public cases?

To begin with, punishment of any infraction that occurred in a
publicly-accessible forum such as Phabricator can be public.  If the
infraction itself can remain public, the punishment for it can also.
That seems like a good starting point.

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] My Phabricator account has been disabled

[Aryeh Gregor]

On Wed, Aug 8, 2018 at 4:01 PM, Alex Monk  wrote:
> So are we supposed to be careful about using 'wtf' now?

I don't think saying "WTF" to someone, especially spelled out, is
usually conducive to eliciting a constructive response from them.
Something like "Could you please explain why you did that?" is pretty
much always a better way to communicate.  (This is if it's directed at
their actions/code/etc., obviously, not at PHP or whatever.)

However, it is not at all clear that such a statement is against
anything written in the CoC.  At most it would be "offensive
comments", which is extremely vague.  I think it would be good if all
forms of non-constructive communication *were* against the CoC --
although not necessarily dealt with by bans -- but I don't see that
they are.

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] Proposal for removal without deprecation of MagicWord::clearCache

[Aryeh Gregor]

On Tue, Jul 31, 2018 at 3:46 PM, Stephan Gambke  wrote:
> I agree that there is a trade-off to be done.
> I disagree about expecting code to be put where it is visible to core
> developers. I do appreciate that you go and look for where the
> functionality that you are working on is used outside core. But
> ultimately MW is publishing a public interface and it is unreasonable
> to expect all consumers of that interface to "register" and publish
> their code as well.

We don't, but if they don't publish it we have to guess at what will
or won't break it, which means there's a higher chance that we will
break it.

> The difference is that in that case the test would have run through,
> throwing just a notice and the call to the deprecated function could
> have been removed another day.

Hard-deprecation fails the test completely, right?

> Of course, if they did not want to deal
> with breakages, that maintainer should not have pulled MW master in
> the first place and should have just worked on whatever MW version
> they left off of a few months ago when they last worked on their
> extension.

Correct, we do not and should not go out of our way to make life easy
for people running unstable MediaWiki.  It's not meant for production
unless you're following development closely and are prepared to deal
with breakage.

> True. I don't know how other people do it, but I usually only scan the
> release notes for big changes and otherwise rely on deprecation notes
> to pop up. Doing otherwise would require me to maintain a register of
> all MW core functions called from my extensions and cross-checking it
> by hand.

Indeed, but as soon as you see the error message, it's easy to check
the release notes/history to figure out what to do.
Deprecation/removal notices there should say what the replacement is
as clearly as possible.  (If they don't, that's a separate problem.)

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] Proposal for removal without deprecation of MagicWord::clearCache

[Aryeh Gregor]

On Tue, Jul 31, 2018 at 10:43 AM, Stephan Gambke  wrote:
> There are three "probability qualifiers" in that sentence (seems, probable, 
> basically). You just don't know if somebody now has to fix their code.

Correct -- I can't know, because they didn't put their code in a place
where code search finds it.  What we can deprecate or remove and how
quickly is always a guess vis-a-vis extensions that we can't see.  I
*do* know that it would be a significant hassle to go through the
deprecation procedure for every ancient crufty method from 2004 (or
whatever) that we want to get rid of.  We would be paying a definite
cost for a gain that might not even exist.

So we have to make a trade-off between the burden on extension
developers and on core developers.  When it seems likely that
basically no non-public extension/skin uses the feature, I think it's
reasonable to say that we should do what's easiest for core
developers.  If a few people out there who didn't put their extensions
where we can see them have to go to some trouble, that's life.

> It is a one-line change from your perspective. For the maintainer it will be 
> picking up the extension after some time, fixing some unrelated bug, running 
> the tests and have them fail. Finding the problem should be easy enough 
> (method not found), but they then have to investigate, why that stupid method 
> is missing all of a sudden and find and understand the recommended solution, 
> digging through commits and phabricator tickets. They then need to apply the 
> solution which, if they want clean self-contained commits, includes 
> committing or stashing their current code, opening a new branch, applying the 
> fix, running the test, merging the branch to master, picking up the original 
> branch and rebasing it. All of this on top of the small bug fix they actually 
> came for.

You're right, but all that is just as true if we had deprecated the
feature per the policy.  You're talking about someone who didn't check
the release notes before upgrading to find out if his extension used
any removed features, so presumably he also doesn't check the release
notes for deprecated features.  Either way, the release notes do say
that it was removed (easily searchable by method name) and what to do
instead.  (Although in this case I think the "what to do instead" in
the release notes could be improved upon.)

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] Proposal for removal without deprecation of MagicWord::clearCache

[Aryeh Gregor]

On Mon, Jul 30, 2018 at 4:28 PM, Stephan Gambke via Wikitech-l
 wrote:
> There have been several proposals for removal without deprecation in the past 
> few days, each with more impact on existing code - from "virtually certain 
> nobody ever used" to "the fix is trivial".
>
> I know it is annoying as heck to keep outdated code, but if you ask me, 
> unless there is an immediate compelling reason for the removal the 
> deprecation policy should not be bypassed. And in my opinion "I did not find 
> any extension that uses this method" is not sufficient. Not all 
> extensions/skins are hosted on WMF servers. And being trivial to fix does not 
> help, either. It would be great if everybody could monitor MW master, run CI 
> tests regularly for all their extensions and fix any breakages. Unfortunately 
> many people do not have the time to do that, so they need the deprecation 
> period (and at times the deprecation popups to even notice that something is 
> wrong).
>
> So, I would request to keep the clearCache() method for the foreseen 
> deprecation period. (And generally speaking, though not explicitly required 
> by the deprecation policy and probably not applicable in this instance, it 
> would be great if deprecated methods would actually be kept in working 
> condition and not reduced to a (literally) empty husk just to stay in line 
> with the letter of the policy.)

I personally have maintained local hacks to various software packages
in the past, and definitely symphathize with the headache of having to
keep up-to-date.  However, the deprecation policy doesn't try to help
people who aren't keeping up-to-date.  It will just postpone their
extension's breakage by two releases, not decrease the overall amount
of work they have to do.  The deprecation policy is just to give
extension maintainers a bit of breathing room so they don't have to
scramble to update their extension before it breaks.

As far as these specific proposals, the wording of the deprecation
policy suggests that removal without deprecation should only be cases
where it's absolutely necessary.  I think we should also allow such
removal when it seems probable that basically nobody actually used the
feature.  For instance, the feature being removed in this case is only
useful for tests of MagicWords.  It doesn't seem worth the hassle to
go through a whole procedure to remove it if it will probably affect
nobody, and almost certainly affect at most one or two people, and the
affected parties will just have to make a one-line change.  I'm not
saying the burden on extension developers will be zero, but a lot more
time will be needed for us to follow the procedure than we'd save
anyone.

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

[Wikitech-l] Proposal for removal without deprecation of MagicWord::clearCache

[Aryeh Gregor]

Patch to remove: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/447647
Bug: https://phabricator.wikimedia.org/T200247
Existing uses: 
https://codesearch.wmflabs.org/search/?q=MagicWord%3A%3AclearCache=nope==

I'm in the middle of creating a MagicWordFactory service to replace
the MagicWord static methods.  This obsoletes the clearCache() method.
Instead, just reset the service with resetServiceForTesting().  Since
clearCache() is used in only three places in public code, and is only
useful to begin with for tests, and the fix is trivial, I propose
removing it without deprecation rather than bothering to maintain code
for compatibility.

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

[Wikitech-l] Proposal for removal without deprecation of OutputPage::getPageTitleActionText() and OutputPage::setPageTitleActionText()

[Aryeh Gregor]

Patch to remove: https://gerrit.wikimedia.org/r/449022
Bug: https://phabricator.wikimedia.org/T200643

Commit message of patch copied here for your convenience:

Remove long-dead OutputPage methods set/getPageTitleActionText()

They were accidentally made non-functional in April 2009 by commit
e4d21170. Subsequent commits 2d045fa1, 9e230f30, e275ea28, ae45908c
removed all callers by October 2011. Needless to say, I found no
callers in core or extensions.

It seems we have the functionality in some other way, probably just by
directly calling setHTMLTitle(), so there's no need to revive this
feature.

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

[Wikitech-l] Proposal for removal without deprecation of OutputPage::addMetadataLink and OutputPage::getMetadataAttribute

[Aryeh Gregor]

Patch to remove: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/447629

In the course of writing more tests for OutputPage, I came across the two
methods addMetadataLink() and getMetadataAttribute() that didn't make sense
to me.  After a bit of digging, I found they were added in 2004 (22f8c4ce)
to support output of Creative Commons and Dublin Core 
tags.  The only thing they do (compared to directly calling addLink()) is
for some reason setting rel="alternate meta" instead of rel="meta" for the
first link.  I didn't find any reason why they should do that, but based on
a nearby comment I speculate it was to work around bugs in some UAs that
were around at that time.

The functionality of outputting these  tags was moved to extensions
in 2011 (27c3b22b), while the methods were left in core.  Those extensions
(CreativeCommonsRdf and DublinCoreRdf) now call addHeadItem() directly.  A
search found no other uses in core or extensions.  Any callers could be
trivially changed to use addLink() or another method.

Legoktm points out that getMetadataAttribute() is marked public, and
therefore strictly speaking can only be removed via the deprecation
policy.  (Although getMetadataAttribute() is really just a helper function,
and the actual public interface is addMetadataLink() AFAICT.)

Practically speaking, although they aren't hurting anything, I think this
is useless cruft that it's virtually certain nobody ever used, so I suggest
skipping the deprecation period in this case.  Does anyone object?
___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: refcounting - which types to use ?

[Aryeh Gregor]

On Thu, Aug 17, 2017 at 7:07 AM, Enrico Weigelt, metux IT consult
 wrote:
> OTOH, is there a way to create versions that really return it as rval,
> so I conveniently could just call like that ?
>
>nsCOMPtr list = addrbook->FindRecipients(addr);

You can do this with a return type of
already_AddRefed.  Then instead of

  list.forget(_retval);
  return NS_OK;

you do just

  return list.forget();

Note that in this case you cannot ignore the return value -- it must
be assigned to an nsCOMPtr or a RefPtr, or else it will leak.  Ideally
we would allow a return type of nsCOMPtr&&, but
currently I think we don't.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Linking with lld instead of ld/gold

[Aryeh Gregor]

On Mon, Aug 14, 2017 at 12:08 AM, Sylvestre Ledru  wrote:
> Packages for lld on Debian & Ubuntu are available on https://apt.llvm.org/

Ubuntu 16.04 and later also has an lld-4.0 package in its own repos
(sudo apt install lld-4.0).
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

[Bug 1709684] [NEW] package libxfont-dev (not installed) failed to install/upgrade: trying to overwrite '/usr/share/doc/libxfont-dev/fontlib.html', which is also in package libxfont1-dev 1:1.5.1-1ubun

[Aryeh Gregor]

Public bug reported:

$ sudo apt -f install
Reading package lists... Done
Building dependency tree
Reading state information... Done
Correcting dependencies... Done
The following packages were automatically installed and are no longer required:
  linux-headers-4.10.0-27 linux-headers-4.10.0-27-generic linux-headers-4.8.0-36
  linux-headers-4.8.0-36-generic linux-image-4.10.0-27-generic 
linux-image-4.8.0-36-generic
  linux-image-extra-4.10.0-27-generic linux-image-extra-4.8.0-36-generic
  linux-signed-image-4.10.0-27-generic snap-confine
Use 'sudo apt autoremove' to remove them.
The following additional packages will be installed:
  libxfont-dev
The following NEW packages will be installed:
  libxfont-dev
0 upgraded, 1 newly installed, 0 to remove and 3 not upgraded.
31 not fully installed or removed.
Need to get 0 B/124 kB of archives.
After this operation, 494 kB of additional disk space will be used.
Do you want to continue? [Y/n] y
(Reading database ... 305731 files and directories currently installed.)
Preparing to unpack .../libxfont-dev_1%3a2.0.1-3~ubuntu16.04.1_amd64.deb ...
Unpacking libxfont-dev (1:2.0.1-3~ubuntu16.04.1) ...
dpkg: error processing archive 
/var/cache/apt/archives/libxfont-dev_1%3a2.0.1-3~ubuntu16.04.1_amd64.deb 
(--unpack):
 trying to overwrite '/usr/share/doc/libxfont-dev/fontlib.html', which is also 
in package libxfont1-dev 1:1.5.1-1ubuntu0.16.04.1
Errors were encountered while processing:
 /var/cache/apt/archives/libxfont-dev_1%3a2.0.1-3~ubuntu16.04.1_amd64.deb
E: Sub-process /usr/bin/dpkg returned an error code (1)

ProblemType: Package
DistroRelease: Ubuntu 16.04
Package: libxfont-dev (not installed)
ProcVersionSignature: Ubuntu 4.10.0-28.32~16.04.2-generic 4.10.17
Uname: Linux 4.10.0-28-generic x86_64
ApportVersion: 2.20.1-0ubuntu2.10
Architecture: amd64
Date: Wed Aug  9 20:45:28 2017
DuplicateSignature:
 package:libxfont-dev:(not installed)
 Unpacking libxfont-dev (1:2.0.1-3~ubuntu16.04.1) ...
 dpkg: error processing archive 
/var/cache/apt/archives/libxfont-dev_1%3a2.0.1-3~ubuntu16.04.1_amd64.deb 
(--unpack):
  trying to overwrite '/usr/share/doc/libxfont-dev/fontlib.html', which is also 
in package libxfont1-dev 1:1.5.1-1ubuntu0.16.04.1
ErrorMessage: trying to overwrite '/usr/share/doc/libxfont-dev/fontlib.html', 
which is also in package libxfont1-dev 1:1.5.1-1ubuntu0.16.04.1
InstallationDate: Installed on 2017-07-26 (13 days ago)
InstallationMedia: Ubuntu 16.04.2 LTS "Xenial Xerus" - Release amd64 
(20170215.2)
RelatedPackageVersions:
 dpkg 1.18.4ubuntu1.2
 apt  1.2.24
SourcePackage: libxfont2
Title: package libxfont-dev (not installed) failed to install/upgrade: trying 
to overwrite '/usr/share/doc/libxfont-dev/fontlib.html', which is also in 
package libxfont1-dev 1:1.5.1-1ubuntu0.16.04.1
UpgradeStatus: No upgrade log present (probably fresh install)

** Affects: libxfont2 (Ubuntu)
 Importance: Undecided
 Status: New


** Tags: amd64 apport-package package-conflict xenial

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/1709684

Title:
  package libxfont-dev (not installed) failed to install/upgrade: trying
  to overwrite '/usr/share/doc/libxfont-dev/fontlib.html', which is also
  in package libxfont1-dev 1:1.5.1-1ubuntu0.16.04.1

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/libxfont2/+bug/1709684/+subscriptions

-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Re: Intent to change editor newline behavior

[Aryeh Gregor]

On Thu, Apr 6, 2017 at 3:34 PM, Ehsan Akhgari  wrote:
> Thanks for the follow-ups and for filing bug 1354060.  But now that we
> know that this affected Gmail, I'll note that the risk around this is
> still there, and while keeping this on the pre-release channels for a
> while reduces the risk somewhat, we should still be super careful when
> we decide to let it go to release, and preferably do so in coordination
> with the Web Compat team and in preparation to deal with any fallout.  :-)

I agree that it makes sense to not let this ride the trains at the
normal pace.  I guess it's Masayuki's decision how exactly to proceed.
(At any rate, it's certainly not mine, thankfully.  :) )
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to change editor newline behavior

[Aryeh Gregor]

On Wed, Apr 5, 2017 at 11:22 PM, Daniel Veditz  wrote:
> One line or a thousand isn't the point; building, release and update
> testing, and shipping 90 locales times 6 platforms is a huge amount work. If
> we have a pref and break something we can back it out easily and quickly. We
> could even do a slow roll-out independent of a release if we're extra
> concerned about compat (as we have done with e10s and sha1 deprecation, for
> example).

Then it certainly sounds good to make this a pref.  I filed bug 1354060.

Incidentally, bug 1353913 proves that Gmail does use our line-breaking
behavior, and so presumably so do other editors.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Enabling Pointer Events in Firefox (desktop) Nightly on Mac and Linux

[Aryeh Gregor]

On Wed, Apr 5, 2017 at 7:34 PM, Tom Ritter  wrote:
> It looks like this exposes pointerType, which reveals whether the user
> is using a mouse, pen, or touch input.
>
> It also exposes detailed information about the geometry of the input
> (size of the thing pointing, pressure, tilt, twist.
>
> All of these are more detailed information that websites currently
> receiving, meaning that this can be used as a mechanism for
> fingerprinting (and tracking) users.

I think this has been discussed here before, but I don't recall a firm
conclusion: has anyone established whether a nontrivial number of
users are non-fingerprintable as things stand?  If the vast majority
of users can be fingerprinted right now, and there are no realistic
plans to change that, it doesn't seem like we should care about
increasing fingerprinting ability.  I haven't investigated, but I'd be
surprised if there are a lot of users who can't be fingerprinted yet,
given the huge and rapidly-expanding number of features in the web
platform.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to change editor newline behavior

[Aryeh Gregor]

On Wed, Apr 5, 2017 at 4:48 PM, Ehsan Akhgari  wrote:
> Originally it seemed that you are working under the assumption that most
> sites are overriding our default newline handling behavior.  This is
> very easy to measure through telemetry by adding a probe for example
> that looks when an HTML editor object is destroyed whether the
> defaultParagraphSeparator command was used to override our default
> behavior, you can send a value of 0 for the false case and a value of 1
> for the true case and we can get a percentage of pages where this
> override actually happens on based on this data.

I think I wasn't clear.  By "overriding" I meant that when the user
presses Enter, they intercept the keystroke and modify the DOM
themselves, and the editor code is never invoked.  I didn't mean that
the sites alter defaultParagraphSeparator.  So we could add a probe
that detects when the editor's Enter-handling code is run at all.

>> If it's not so low, however, we'd need
>> to look at the actual sites to see if they break.  Can we do that
>> somehow through telemetry, or is it a privacy problem?
>
> Detecting the actual breakage that can happen is much more difficult in
> code because you would need to first define what the breakage would
> result in to try to detect it in code which is extremely difficult in
> this case.

If we had a list of top sites that used our Enter-handling code, we
could test them manually.  (But such testing would be unlikely to be
comprehensive.)

> Getting the kind of data I'm suggesting above _now_ that the patch is
> landed seems rather pointless.  There seems to be a lot of disagreement
> on the degree of the risk involved in this change in the first place,
> and until we agree on the level of risk arguing about the details like
> this may be pointless.

It's not pointless if we pref it only to Aurora, and leave it there
until we have more data.

> At the end of the day, this is Masayuki's call.  I certainly understand
> that in the past with changes like this we've had a lot of difficulty
> detecting regressions before the patches have hit the release channel,
> so I'm not sure how much keeping the patch on pre-release channels would
> really help.  :-(  But to me it seems like the kind of thing that we'd
> want to be able to quickly turn off on the release channel through
> shipping a hotfix add-on that sets a pref if something goes wrong...

FWIW, changing the default back to  is a one-line change already.

> Perhaps I'm missing something about the nature of what changed here.
> How is this seldom used?  Am I misunderstanding that the change happened
> was how *Gecko* reacts to the user pressing Enter by default in a
> contenteditable section?  It's true that some editing widgets override
> this behavior somehow, but I'd be really shocked if that's true 100% of
> the time, so I don't understand how you argue this is a seldom used
> feature...

Yes, I do mean that AFAIK high-profile editing frameworks do override
this kind of behavior.  Johannes Wilm seems to be the founder of the
"Fidus Writer" editor project, and when I asked "Do editors all
override hitting Return, Backspace, etc.?" he said "Those involve
taking apart and merging paragraphs, so I cannot really imagine how
one would create an editor without doing that. At the very least one
will need to monitor what the browser does and then adjust if needed."
   The
second sentence is what we have to worry about, though.

One thing I know for sure is, when I looked into it several years ago,
execCommand() usage among high-profile editors seemed to be
nonexistent.  Once you're writing all the code yourself to handle
styling and block formatting and so on, overriding the behavior for
hitting Enter is a no-brainer.

It wouldn't be hard to test whether various sites use our built-in
linebreak behavior when the user hits Enter.  Just put a printf in the
relevant function and see if it gets hit.  Maybe I'll do that.

> I guess I'm personally coming from the perspective of having bitten by
> regressing various things about the editor behavior too many times in
> the past, and every time it happened, we could make an argument about
> how it's extremely unlikely to happen beforehand.  :-)

Yes, on reconsideration, I expect some fallout.  Phasing this in more
cautiously seems like a good idea to me.  I think it might be best to
just leave it in Aurora and Beta for longer than usual before
promoting it.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to change editor newline behavior

[Aryeh Gregor]

On Wed, Apr 5, 2017 at 3:12 AM, Ehsan Akhgari  wrote:
> Exactly.  We can hypothesize either way, but we certainly can't know easily
> without getting some data first.  But unfortunately it's not possible to
> collect data about what sites are doing in terms of DOM fix-ups like this.
> We can, at least, collect data about whether they are overriding the newline
> behavior wholesale though.  Is there any reason why we should not at least
> first collect this data before changing the behavior here?

What exact data would you want?  We could collect data on how often
our built-in newline behavior is used, and if it's low enough we'd be
confident the change is safe.  If it's not so low, however, we'd need
to look at the actual sites to see if they break.  Can we do that
somehow through telemetry, or is it a privacy problem?

If someone has suggestions for the exact telemetry probe to use here,
I'm happy to add one, and maybe keep the change in Aurora until we get
data to make a decision.  I'm not familiar enough with telemetry
(either the theoretical options or our implementation) to decide what
the right probe is.

On Wed, Apr 5, 2017 at 3:31 AM, Benjamin Smedberg  wrote:
> I agree that it doesn't seem likely that telemetry can answer this sort of
> question. However, we could collect data! Pick N top editing tools and
> actually test their behavior. We probably can't get full confidence, but we
> can get a much better picture of the risk involved. If we break (or
> significantly change behavior) on five sites out of 40, that's a strong
> indicator that we're going to have problems.
>
> As an example, have we already tested or is it in a plan to test:
> google docs
> basic and rich text editors on wikipedia
> office 365
> github editor
> common rich text editor libraries, and common CRM software (I don't have a
> list)
> the top hosted email sites: gmail, yahoo, hotmail/outlook, aol, icloud,
> yandex
>
> Being able to assert, before landing this change, that it doesn't break any
> of these sites, would really help in terms of making assertions about the
> risk profile.

I did not test this specific change on those sites.  However, some
years ago I did research execCommand() usage on the web, and found
that high-profile richtext editors essentially didn't use it, because
of browser incompatibilities.  Instead, they wrote their own
functions.  The same incompatibilities exist in newline behavior, so
it's reasonable to say that they would write their own functions for
that too.

This is also supported by a discussion I had with a couple of
developers of major richtext editing libraries (I don't remember which
offhand).  They said that changes like this make no difference to
them, because they don't use the functionality anyway.  They're
interested in fixes in things like selection handling, which they do
use, and features that allow them to more easily override browser
behavior.

Also anecdotally, in terms of dealing with editor bugs like this --
the reports most often come from Thunderbird.  Maybe Ehsan or Masayuki
could weigh in too, but I think that we get very few bug reports in
these parts of the editor from real-world websites.  (We do get some
reports in other parts of the editor, like selection/focus handling.)
This also suggests websites aren't actually using this code much.
(Although maybe it means they just work around our bugs already.)

In fact, I dropped this patch set for a while because the feature is
seldom-used enough on the web that it doesn't seem worth fixing.  I
get the impression that Ehsan shares this point of view.

It's also worth noting that contenteditable is a very complicated
feature to use in real life, particularly given browser
incompatibilities, and I think almost all sites that use it are either
very large or use one of the major rich-text editing libraries.  If
I'm correct, then we don't have to worry so much about breaking a long
tail of small sites that won't get reported quickly.  If Gmail,
Wikipedia, TinyMCE, etc. break, we (or they) are likely to get reports
soon enough.  Large sites are also usually well-maintained and do
their own testing, and will fix the issue quickly.  (But if a library
breaks, or software like MediaWiki, there will be a long tail of old
sites that will remain broken even after the library is fixed, because
they keep using old versions of the library.)

So that's my reasoning for why I don't think this is *so* risky.  But
I agree that I don't really know.  As I said, I'd be happy to let this
stay on Aurora or beta for longer, and/or add some telemetry (if
someone tells me what telemetry we want).
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to change editor newline behavior

[Aryeh Gregor]

On Tue, Apr 4, 2017 at 5:57 PM, Ehsan Akhgari  wrote:
> I don't own this module any more, so this isn't my call to make, but if
> I had to choose what to do here, I would probably either choose to not
> change our behavior (since I'm not sure what we're gaining here
> concretely -- as AFAIK we're not investing in bringing our behavior on
> par with other engines on a more broad basis with regards to editing),

Masayuki seems to be in favor of trying to match Blink more.

> or at the lack of that, adding some telemetry first to get data on how
> often the defaultParagraphSeparator command is used in the wild, since
> AFAICT your change is basically only Web compatible on the assumption
> that this command is used quite heavily.

I doubt it's used much.  My assumption is only that not many sites are
UA-sniffing Firefox, finding the s, and modifying them in some way
that breaks if they're no longer s.  That could still be totally
wrong, though!

> On the idea of the test plan that Benjamin brought up, I'm not sure what
> to put in such a test plan, due to the issue I mentioned above (it being
> totally non-obvious what the expected breakage of this change would look
> like.)

We could put the default defaultParagraphSeparator change behind a
pref and leave the pref off on release (or on beta and release?) for
some period and see if we get bug reports.  I don't think there's any
way to detect breakage by telemetry, so we'd have to rely on user
reports.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to change editor newline behavior

[Aryeh Gregor]

On Mon, Apr 3, 2017 at 11:39 PM, Benjamin Smedberg
 wrote:
>
> I'd like to encourage you to set up a test plan for this. My impression of 
> the risk profile of this work is that we could easily break some really 
> important use-cases, and it's likely that sites customize for gecko behavior 
> and rely on it, either accidentally or on purpose. This is definitely the 
> kind of thing that would be worth rolling out carefully and perhaps slowly. 
> Will this behavior be behind a pref which is easy to flip, test, and roll out?

As implemented, it is not behind a pref.  Masayuki didn't request a
gradual rollout, so I pushed to inbound already.  I suspect it will
not actually break anything, because sites that use the editor
normally avoid browser compatibility issues here by completely
overriding the newline behavior, so they will be unaffected.  If any
sites actually do break from this, I would be very interested to have
the data that people are really using our default newline behavior!

I'd like to know what Ehsan and Masayuki think about the likely compat
impact here.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Intent to change editor newline behavior

[Aryeh Gregor]

In our rich-text editor (used in Firefox for designMode and
contenteditable), when the user hits Enter, we have historically always
inserted a .  This does not match any other browsers, which use  or
 as line separators.  In bug 1297414, I'm changing our behavior to use
 as a line separator.  This matches Blink/WebKit.

So if you have the text "foobar" and hit Enter in between the "foo" and the
"bar", previously you would get "foobar", and soon you will get
"foobar".  The defaultParagraphSeparator command can
be used to change the separator to "p" instead (which matches Edge's
default behavior last I checked).

Pages or embedders that want to keep the old behavior can run the following
command: document.execCommand("defaultParagraphSeparator", false, "br").

This change is not likely to affect high-profile sites that use rich-text
editing (webmail etc.), because due to browser incompatibility, these sites
all override this behavior anyway.

Our new behavior is as specified in the essentially unmaintained editing
specification that I wrote several years ago, and tested by the
web-platform-tests editing suite.  (Except that the "br" value to
defaultParagraphSeparator is unspecified, and is a Mozilla-specific
extension for backwards compatibility.)
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Removing navigator.buildID?

[Aryeh Gregor]

On Mon, Oct 31, 2016 at 4:00 PM, Henri Sivonen <hsivo...@hsivonen.fi> wrote:
> On Mon, Oct 31, 2016 at 3:01 PM, Aryeh Gregor <a...@aryeh.name> wrote:
>> If the concern is fingerprinting, perhaps it could be exposed only to
>> sites that the user is logged into (assuming we have a good working
>> definition of "logged in")?
>
> I think that's over-engineering it.
>
> I suggest we make it behave the current way for chrome,
> https://*.mozilla.org and https://*.netflix.com origins and make it
> return a constant otherwise.

This only helps us and Netflix.  Other sites that want to track fixes
will be left out in the cold.

Taking a step back: is fingerprinting really a solvable problem in
practice?  At this point, are there a significant fraction of users
who can't be fingerprinted pretty reliably?  Inevitably, the more
features we add to the platform, the more fingerprinting surface we'll
expose.  At a certain point, we might be taking away a lot of features
for the sake of trying to stop the unstoppable.  (I have no idea if
this is actually true now, though.  This is a genuine question.  :) )
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Removing navigator.buildID?

[Aryeh Gregor]

On Mon, Oct 31, 2016 at 10:02 AM, Chris Peterson  wrote:
> Alternately, buildID could be hidden behind a domain whitelist pref, seeded
> with sites working with Mozilla. If you are, say, a Netflix customer, they
> already know who you are, so exposing your buildID to them is not much of a
> tracking risk. :)

If the concern is fingerprinting, perhaps it could be exposed only to
sites that the user is logged into (assuming we have a good working
definition of "logged in")?
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to restrict to secure contexts: navigator.geolocation

[Aryeh Gregor]

On Tue, Oct 25, 2016 at 9:43 PM, Eric Rescorla  wrote:
> Setting aside the policy question, the location API for mobile devices
> generally
> gives a much more precise estimate of your location than can be obtained
> from the upstream network provider. For instance, consider the case of the
> ISP upstream from Mozilla's office in Mountain view: they can only localize
> a user to within 50 meters or so of the office, whereas GPS is accurate to
> a few meters.

Yes, but the privacy impact of such an increase in precision is much
less than if you only knew geo IP data.

> And of course someone who is upstream from the ISP may just
> have standard geo IP data.

Are there real-world MITMs who are upstream from the ISP?
(Governments presumably compromise the ISP itself by court order or
equivalent.)
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to restrict to secure contexts: navigator.geolocation

[Aryeh Gregor]

On Tue, Oct 25, 2016 at 8:12 PM, Anne van Kesteren  wrote:
> The basic problem is prompting the user at all for non-HTTPS since
> that leads them to think they can make an informed decision whereas
> that's very much unclear. So prompting more would just make the
> problem worse.
>
> We want to get to a place where when we prompt the user on behalf of a
> website we have some certainty who is asking the question (i.e.,
> HTTPS).

By that logic, we should not permit users to submit forms to non-HTTPS
either.  I agree that if we were designing the web from scratch we
would absolutely require HTTPS for everything, but in reality we have
to make a cost-benefit analysis in each case.  That means analyzing
the threats to our users' privacy or security and deciding whether it
outweighs the user annoyance.  If the prospect of a privacy leak is
implausible or not a big privacy compromise, it doesn't necessarily
outweigh the cost of aggravating users.  I don't think that privacy or
security issues are exempt from cost-benefit analysis like any other
feature or bug fix -- they're unusually important, but still do not
have infinite value.

In this specific case, it seems that the usual candidates for MITMing
(compromised Wi-Fi, malicious ISP) will already know the user's
approximate location, because they're the ones who set up the Internet
connection, and Wi-Fi has limited range.  What exactly is the scenario
we're worried about here?
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to implement and ship: SVGElement.prototype.dataset

[Aryeh Gregor]

On Thu, Aug 11, 2016 at 12:53 AM, Boris Zbarsky  wrote:
> Summary: HTML elements have a .dataset property that allows convenient
> somewhat structured access to data-* attributes.  The proposal is to add
> this to SVG elements too, following corresponding changes in the SVG
> specification.

Why doesn't this just go on Element, like .className and .id?  Is
there any reason we would want these any less on MathML or whatever
(supposing anyone were to actually use them)?
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Return-value-optimization when return type is RefPtr

[Aryeh Gregor]

On Mon, Aug 15, 2016 at 3:55 PM, Aryeh Gregor <a...@aryeh.name> wrote:
> IMO, it makes sense to move ahead with this.

One more point I forgot to (re-?)mention: with proper move
constructors, it's not just already_AddRefed return values that
should be changed to RefPtr, but also T* return values, in the case
where the returning function already had a RefPtr lying around.
This will *reduce* addref/release in common cases, like:

  T*
  Foo()
  {
RefPtr t = DoStuff();
return t;
  }

  RefPtr t = Foo();

Currently this does a release when Foo() returns, and a new addref
when assigning to the new variable.  If Foo() instead returned
RefPtr with proper move constructors, these would be removed.  You
also could not assign the return value of Foo() to a raw pointer
(without .get()), which improves security against use-after-free
without any extra addref/release.  (The release is just handled by the
caller instead of callee.)

It could inadvertently introduce extra addref/release if a function
that doesn't hold a reference on some code paths has its return type
changed to RefPtr, like:

  T* Bar();

  T*
  Foo()
  {
return Bar();
  }

  Foo()->Method();

If Foo()'s return type was changed to RefPtr, this would silently
add an addref/release.  I don't know how to prevent this.  Would
making the relevant constructor explicit do it?  If so, would it cause
issues in other places?

Also -- now I'm remembering more of the issues here -- this would also
require .get() to pass a function's return value directly as a
function parameter, unless bug 1194215 is fixed (see also bug
1194195).  It's not *so* common to do this, but it's common enough
that we don't want to require .get(), I think.

So this isn't such a no-brainer, but I think it is where we ideally want to be.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Return-value-optimization when return type is RefPtr

[Aryeh Gregor]

Sorry for the necroposting, but some points that I think are still
worth correcting:

On Thu, Jun 16, 2016 at 11:50 AM, Boris Zbarsky  wrote:
> On 6/16/16 3:15 AM, jww...@mozilla.com wrote:
>>
>> I think that is the legacy when we don't have move semantics. Returning
>> RefPtr won't incur ref-counting overhead and is more expressive and
>> functional.
>
>
> Except for the footgun described in
> , yes?

This is fixed for (ns)RefPtr in
, and nsCOMPtr
in .  The
footgun code no longer compiles, because Foo() returns RefPtr&& and
will not convert implicitly to T*.

On Thu, Jun 16, 2016 at 5:25 PM, Eric Rescorla  wrote:
> On Thu, Jun 16, 2016 at 2:15 PM, Michael Layzell 
> wrote:
>
>> We pass T* as an argument to a function too often for this to e practical.
>>
>
> Can you explain why? Is your point here to avoid people having to type
> .get() or the mass
> conversion from the current code? The former seems like it's a feature
> rather than a bug...

.get() is basically dangerous and IMO people should not be used to
writing it everywhere.  Passing the pointer from a local RefPtr to a
function without addreffing is not dangerous, because the callee
cannot remove your local strong reference and the caller is guaranteed
to hold the reference until the callee returns.  We should not have to
use .get() everywhere just because the compiler isn't smart enough to
figure out automatically that the operation is actually safe.

I had a proposal (with implementation) for some new parameter types to
replace raw pointers, which would hopefully allow complete removal of
automatic conversion to raw pointer without requiring extensive
.get(), and would also enforce other safety constraints (prohibiting
passing pointers that you don't hold a strong reference to).

  But froydnj
decided he wasn't willing to take them at the present time (and I'm
not sure I disagree).

All of these features are built into Rust, incidentally, because it
tracks lifetimes and will automatically figure out which usages are
safe.

Mass conversion is not an absolute blocker if the change is valuable
enough.  Somebody might know how to automatically rewrite the code,
and if not, I've done pretty large conversions by hand in the past.
(It's just hours of repeatedly recompiling and fixing the same thing
over and over again.)

On Thu, Jun 16, 2016 at 8:40 PM, smaug  wrote:
> Can we somehow guarantee that RVO kicks in? It feels quite error prone to
> rely on compiler optimization when dealing with
> possibly slow Addref/Release calls.

No, RVO is never guaranteed, but move constructors are guaranteed to
be used instead of copy constructors whenever you're returning a local
variable.  Thus we only need to make sure that all cases have move
constructors (including things like moving nsCOMPtr to RefPtr).
We can write tests to make sure that there are no unexpected
AddRef/Release.  I think the required move constructors don't yet
exist, or at least they didn't last I checked, but they could be added
easily.

IMO, it makes sense to move ahead with this.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Editor module proposal

[Aryeh Gregor]

On Wed, Aug 3, 2016 at 6:01 AM, Masayuki Nakano  wrote:
> It might be better to add Aryer Gregor and/or Jorg K as a peer, though. But 
> I'm not sure what they think.

I could probably review some patches if you want me to.  I'm not
available to review patches the large majority of the year, but I
disable review requests on Bugzilla when I'm away, so people should be
able to figure out how to direct them (although I don't know if
MozReview knows about the disabling?).  I'll definitely have to
forward some reviews to you or ask questions about them, though.  So
if you want to list me as a peer I guess I'd be okay with it.
___
governance mailing list
governance@lists.mozilla.org
https://lists.mozilla.org/listinfo/governance

Re: Intent to ship: invalid values behave like "metadata", not "subtitles"

[Aryeh Gregor]

On Sun, May 8, 2016 at 9:15 AM, Simon Pieters  wrote:
> httparchive (494,168 pages):
>
> SELECT COUNT(*) AS num, REGEXP_EXTRACT(LOWER(body),
> r']+\s)?kind\s*=\s*([a-z]+|["\'][^"\']+["\'])') as match
> FROM [httparchive:har.2016_04_15_chrome_requests_bodies]
> GROUP BY match
> ORDER BY num DESC
>
> Row num match
> 1   17616286null
> 2   523 "subtitles"
> 3   108 "captions"
> 4   58  "metadata"
> 5   6   "subtitle"
> 6   6   'subtitles'
> 7   5   "thumbnails"
> 8   3   'captions'
> 9   1   "dotsub"
> 10  1   "${assettracktype}"
> 11  1   'subtitle'
>
>
> We could add "subtitle" as a new keyword if that turns out to be a problem.

Thanks for the data!  Looks like we're talking on the order of 0.001%
of pages, so I think this can be safely landed.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to ship: invalid values behave like "metadata", not "subtitles"

[Aryeh Gregor]

On Thu, May 5, 2016 at 4:32 PM, Florin Mezei
 wrote:
> Do you still intend to do some analysis to see whether this will be a
> problem in real life? We have somewhat of a history in shipping changes that
> break compatibility, and then end up doing dot releases (in some cases).

I wasn't planning on putting in the work unless others thought it was
really necessary.  In the linked bug, Boris wasn't sure.  I don't know
anything about the feature's deployment, but I'd venture to guess that
it's not so widespread that the number of misuses would be too
significant in absolute terms.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Intent to ship: invalid values behave like "metadata", not "subtitles"

[Aryeh Gregor]

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

Relevant change in standards:
https://github.com/whatwg/html/issues/293, also maybe
https://www.w3.org/2015/10/28-htmlcue-minutes.html

Bugs filed against other UAs:
https://bugs.chromium.org/p/chromium/issues/detail?id=608772
https://bugs.webkit.org/show_bug.cgi?id=157311
https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/7426340/

Invalid values for  are currently treated as "subtitles"
by all UAs, pursuant to old specs.  This means that if we want to add
any new values in the future, old UAs will treat them as subtitles,
which might result in undesired behavior.  As such, several months
ago, the spec was changed to treat invalid values as "metadata", so
UAs will ignore unrecognized values.  No UA has yet implemented the
new spec.

The risk in implementing this is that sites might be inadvertently
using invalid values right now and relying on the fact that they get
treated as "subtitles".  I haven't done any telemetry or other
analysis at this point to gauge if this will be a problem in real
life.

If there are no objections, I intend to land this change on Sunday.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Can MAX_REFLOW_DEPTH be increased?

[Aryeh Gregor]

On Mon, May 2, 2016 at 9:58 PM, Boris Zbarsky  wrote:
> For a 900KB stack (what we have on Windows, per
> )
> 512 DOM nodes deep corresponds to ~2KB per DOM node.  200 deep corresponds
> to 5KB.
>
> Now quick, tell me how much stack space MSVC with PGO is using for the
> combination of ElementRestyler::Restyle and ElementRestyler::RestyleChildren
> and ElementRestyler::RestyleContentChildren (which is the unit of recursive
> work when recalculating styles down the tree).  How much stack space will it
> use with the next MSVC release?  How much stack space will it use if we
> change the ElementRestyler struct some (note that this goes on the stack
> before the Restyle() call on it)?
>
> The answer at least for me is that I don't know.  But I bet it's quite
> specific to particular code paths and compilers

Sure.  The same is potentially true if the stack depth limit is 200.
Did anyone do all these calculations to determine that was safe, or
did they just pick a number and see what worked?  By your logic, maybe
the next compiler version will use more stack space and suddenly we'll
start crashing at 200 nodes deep also.

If people are concerned about the possibility that a few weird pages
that are already broken might crash, and nobody wants to do the work
to check this, maybe Chrome (and Safari?) are willing to reduce their
limit to 200.  I don't see why they'd object.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Can MAX_REFLOW_DEPTH be increased?

[Aryeh Gregor]

On Mon, May 2, 2016 at 9:19 PM, Steve Fink  wrote:
> It makes me nervous to try to make the overflow behavior the same across
> engines, because it could end up restricting implementation choices. But
> making things roughly the same without trying to make them too much the same
> seems reasonable. And it does sound like there are situations where we just
> outright fail where other browser render *something*, at least, and those
> seem worth improving.

What's the problem with standardizing completely if we pick the limit
to be low enough that we have plenty of room?  If 512 is too high for
us to commit to, we could ask Chrome to drop to 200.  We could also
always change behavior (and maybe the spec) later if it was really
causing a problem.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Can MAX_REFLOW_DEPTH be increased?

[Aryeh Gregor]

On Mon, May 2, 2016 at 8:51 PM, L. David Baron  wrote:
> So, at the very
> least, limiting in the parser isn't effective anymore and we need
> checks at later stages, which hopefully could be done in a
> standardizable way.

Having dynamic fallback checks for anything not currently standardized
is of course perfectly sensible.  But we should standardize what we
can.

If the idea is to stop stack overflow, it doesn't make sense to me to
have the check in the parser at all.  It should be on the DOM level.
Otherwise, scripts could make an arbitrarily deep stack, like this:



var cur = document.body;
for (var i = 0; i < 1; i++) {
  var child = document.createElement("span");
  cur.appendChild(child);
  if (child.parentNode != cur) {
break;
  }
  cur = child;
}
document.body.textContent = i;


Which outputs 1.  And indeed, removing the last line of the script
causes an immediate tab crash both in Firefox and Chrome.  So if this
is a security issue, we're vulnerable right now.  If not, we should
probably just standardize the parser limit.  If Firefox and Chrome
both have parser depth limits but not script-exposed depth limits,
probably that's because a non-negligible number of sites historically
hit the parser depth limits, but scripts haven't been much of an
issue.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Can MAX_REFLOW_DEPTH be increased?

[Aryeh Gregor]

On Mon, May 2, 2016 at 8:07 PM, Bobby Holley  wrote:
> In general, dynamic stack checks (measuring the top of the stack at XPCOM
> startup, and comparing it with the stack at the point of interest) seem
> preferable to hard-coding number-of-recursive-calls, since it doesn't
> depend on the size of stack frames, which may drift over time. We can't do
> this for JS (see the comments surrounding the MXR link above), but I bet we
> could for layout.

I think we should aim to not make page rendering depend on things like
the size of stack frames or the OS-provided stack size.  :)  We should
pick a value that all UAs can commit to comfortably supporting and
standardize it (as well as standardizing the behavior for how to
process the non-conformant page).  Then if authors write weird pages,
they'll break identically in all browsers.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Can MAX_REFLOW_DEPTH be increased?

[Aryeh Gregor]

On Mon, May 2, 2016 at 10:22 AM, Henri Sivonen  wrote:
> If the depth limit is still needed, can now be increased?

What do other UAs do?  This seems like it may as well be standardized,
just so the odd corner-case page breaks the same in all UAs.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to ship: ParentNode.prepend(), ParentNode.append(), ChildNode.before(), ChildNode.after(), ChildNode.replaceWith()

[Aryeh Gregor]

On Tue, Apr 19, 2016 at 5:13 PM, Boris Zbarsky  wrote:
> We could try to mitigate through searching web content for the relevant
> method names, but I believe that was already done when writing the spec.  I
> could be wrong, of course
>
> Past that, I'm not sure how to design a test plan that would catch issues,
> if any, here.  :(

Couldn't we add telemetry probes to alert when they hit circumstances
that would cause a problem?
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Moving XP to ESR?

[Aryeh Gregor]

On Mon, Apr 18, 2016 at 9:02 PM, Chris Peterson  wrote:
> OTOH, if an XP users doesn't mind running an unpatched OS, then they
> probably won't care/know about running an unpatched Chrome browser.

Gmail nags you if you use an outdated Chrome version.  I know someone
who asked me to take a look at their computer because Gmail had a
banner at the top for a few months saying their browser was outdated.
Turns out they were using 32-bit Linux, which Chrome no longer
supports, so I advised them to upgrade to 64-bit at their convenience.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Coding style for C++ enums

[Aryeh Gregor]

On Fri, Apr 8, 2016 at 8:35 PM, smaug  wrote:
> enum classes are great, but I'd still use prefix for the values. Otherwise 
> the values look like types - nested classes or such.
> (Keeping my reviewer's hat on, so thinking about readability here.)

In some cases I think it's extremely clear, like this:

  enum class Change { minus, plus };

whose sole use is as a function parameter (to avoid a boolean
parameter), like so:

  ChangeIndentation(*curNode->AsElement(), Change::plus);

In cases where it might be mistaken for a class, it might make more
sense to prefix the enum class name with "E".  I don't think this
should be required as a general policy, though, because for some enums
it might be clear enough without it.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Maintaining the en-US dictionary that ships with Mozilla products

[Aryeh Gregor]

On Mon, Dec 28, 2015 at 9:31 PM, Jörg Knobloch  wrote:
> Thirdly, the add-on dictionary contains 13% more words than the Mozilla 
> maintained dictionary, and I think in dictionaries, bigger is better.

This is not always true for spelling correction, because there may be
common words that can be misspelled the same as uncommon words.  E.g.,
the word "fro" is perfectly good English ("to and fro"), but in real
user input it's probably a misspelling of "for", and it's not obvious
that it's good to have in a spellchecking dictionary.  Likewise,
uncommon variant spellings are best omitted.  Uncommon words that are
not likely to be misspellings of any more common words are good to
have.

I don't have any strong feelings about the main point of your post.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Intent to unship: DocumentType.internalSubset

[Aryeh Gregor]

The current DOM standard says DocumentType.internalSubset should be removed:

https://dom.spec.whatwg.org/#dom-documenttype-internalsubset

It looks like Chrome has already removed it:

https://code.google.com/p/chromium/issues/detail?id=272096
http://codereview.chromium.org/282823004

It seems the change stuck, because my Chrome 45 doesn't support it.
This suggests it's safe to remove.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

C++ feature proposal: specialize conversions for type of usage (local, member, parameter, etc.)

[Aryeh Gregor]

In bug 1193762, there's been work on eliminating the implicit
conversion from nsCOMPtr to T*, at least for rvalues, to avoid the
safety problem discussed there.  The problem is that then you can't
pass an nsCOMPtr&& to a function that wants a T*, even though this
is perfectly safe: the lifetime of the temporary is guaranteed to
outlast the function call.  The only solution hitherto devised other
than requiring lots of .get() is to invent a new type for refcounted
function parameters, which is awkward.

A new language feature could be used to solve this: allow conversion
operators to behave differently based on how the variable is declared.
For instance, it might convert differently if the source or
destination is a local variable, global/static variable, member
variable, or function parameter.  This would allow our problem to be
easily solved by defining something in nsCOMPtr like:

  operator T* [[parameter]]()&&;

while leaving the operator deleted for non-parameters.

If this can be declared on any method, or perhaps on the class itself,
it could also be used to enforce nsCOMPtr not being used as a global
or static variable.  Are there other places where this would be
useful?  I don't know if this makes sense to propose as a language
feature, but I thought it would be worth bringing up in case anyone
has more compelling use-cases.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: C++ feature proposal: specialize conversions for type of usage (local, member, parameter, etc.)

[Aryeh Gregor]

On Sun, Oct 11, 2015 at 2:09 PM, Aryeh Gregor <a...@aryeh.name> wrote:
> A new language feature could be used to solve this: allow conversion
> operators to behave differently based on how the variable is declared.
> For instance, it might convert differently if the source or
> destination is a local variable, global/static variable, member
> variable, or function parameter.  This would allow our problem to be
> easily solved by defining something in nsCOMPtr like:
>
>   operator T* [[parameter]]()&&;
>
> while leaving the operator deleted for non-parameters.

Actually, you could perhaps do even better than that.  Use nsCOMPtr
for refcounted parameters instead of T*, and then delete the T*
constructor for [[parameter]] nsCOMPtr, and have the constructor
and destructor for nsCOMPtr parameters not do addref/release.  Then
you have the effect of bug 1194195 without having to introduce a new
type.  nsCOMPtr(/RefPtr) would be the type to use anywhere you want to
require a strong reference, and it would magically sort out the
addrefs/releases for you.  (Although I know a lot of people don't like
such magic either, so this isn't strictly superior.  But the option
would be available.)
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Use of 'auto'

[Aryeh Gregor]

On Sun, Aug 2, 2015 at 8:37 PM, Joshua Cranmer  pidgeo...@gmail.com wrote:
 I've discussed this several times, but with we added operator T*()  =
 delete;, that would prevent the scenario you're talking about here.

And FWIW, I have patches that I'm going to shortly submit to bug
1179451 that do this for nsRefPtr (nsCOMPtr is next).
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: mozilla::TemporaryRef is gone; please use already_AddRefed

[Aryeh Gregor]

On Tue, Jul 7, 2015 at 6:10 PM, Michael Layzell mich...@thelayzells.com wrote:
 No, I never checked if it happens on an optimized build, but as C++ follows
 an as-if principal, which means that code has to execute as if those
 temporaries had been created. Unfortunately, AddRef and Release are virtual,
 which, I'm pretty sure, means that the compiler can't optimize them out as
 they may have arbitrary side effects :(.

 So the temporaries are probably eliminated, but the calls to AddRef and
 Release are probably not. I could be wrong on this though.

This is true except in special cases where copy elision is allowed, in
which case the temporary may be removed entirely, with its constructor
and destructor.  According to 12.8.32 of
http://www.open-std.org/Jtc1/sc22/wg21/docs/papers/2011/n3242.pdf,
one of those cases is

when a temporary class object that has not been bound to a reference
(12.2) would be copied/moved to a class object with the same
cv-unqualified type, the copy/move operation can be omitted by
constructing the temporary object directly into the target of the
omitted copy/move

I'm not an expert in C++-ese, but this looks like exactly that, so
compilers should be allowed to remove the AddRef/Release.  Whether
they do or not is a separate question -- I'd hope so, in such a
simple-looking case.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: A question about Range#insertNode

[Aryeh Gregor]

Looks like you're correct, that bit should be removed.  If you update
the test to match the new algorithm (assuming I wrote a test for it,
I'm pretty sure I did) and it doesn't change which browsers pass, then
we could be extra super sure.  :)

On Tue, Jul 7, 2015 at 4:27 PM, Anne van Kesteren ann...@annevk.nl wrote:
 On Tue, Jul 7, 2015 at 12:48 PM, Ms2ger ms2...@gmail.com wrote:
 Step 7 of https://dom.spec.whatwg.org/#concept-range-insert reads

 If /range/’s start node is a Text node, split it with offset
 /range/’s start offset, set /referenceNode/ to the result, and set
 /parent/ to /referenceNode/’s parent.

 In this case, /referenceNode/ has been set to /range/’s start node
 (step 3), and /parent/ has been set to /referenceNode/’s parent (step
 5). The split algorithm returns a node with the same parent as its
 argument, so I believe that this step doesn't actually change /parent/.

 Could you confirm and maybe add a note to the spec here?

 You mean removing setting /parent/ again? Seems conrrect. I wonder if
 Aryeh has a second to take a look since I believe he wrote that
 algorithm.


 --
 https://annevankesteren.nl/

Re: mozilla::TemporaryRef is gone; please use already_AddRefed

[Aryeh Gregor]

On Fri, Jul 3, 2015 at 6:22 PM, Michael Layzell mich...@thelayzells.com wrote:
 So the ternary actually causes an unnecessary AddRef/Release pair, neat.

Did you check whether this actually occurs in an optimized build?  C++
allows temporaries to be optimized away under some circumstances,
e.g., when returning a local variable.  It would make a lot of sense
to me if it allowed the temporary created by a ternary operator to be
optimized away.

 The problem here appears to be that when deciding the type of the ternary,
 c++ chooses nsRefPtrFoo, rather than Foo*. Adding the get() makes C++
 choose the correct type for the ternary, and avoids the cast of the rvalue
 reference.

I think it's pretty clear that nsRefPtrFoo must be the type of the
ternary expression.  I would be rather surprised if the type of the
ternary depended on what you did with the result!
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: mozilla::TemporaryRef is gone; please use already_AddRefed

[Aryeh Gregor]

On Fri, Jul 3, 2015 at 3:56 PM, Neil n...@parkwaycc.co.uk wrote:
 Aryeh Gregor wrote:

 we still want a new type for function parameters that accepts implicit
 conversions from nsRefPtr/nsCOMPtr, to use instead of raw pointers.

 Sure, but that won't stop someone from writing ArgFoo foo = ReturnFoo2();
 will it?

No, but that's an extremely obvious bug that should be caught in
review.  If you want to be sure, you could do static analysis of some
kind, but I don't expect it's necessary.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: mozilla::TemporaryRef is gone; please use already_AddRefed

[Aryeh Gregor]

On Thu, Jul 2, 2015 at 3:50 PM, Neil n...@parkwaycc.co.uk wrote:
 Would you mind reminding me what the failure case this avoids is?

already_AddRefedFoo
ReturnFoo1()
{
  nsRefPtrFoo foo = new Foo();
  return foo.forget();
}

nsRefPtrFoo
ReturnFoo2()
{
  return new Foo();
}

// This doesn't compile
Foo* foo = ReturnFoo1();

// This happily compiles and causes use-after-free
Foo* foo = ReturnFoo2();


Naturally, the hard-to-catch case is when the function returns
something that usually has a refcount above one, so it works fine, but
sometimes will return something with a refcount of exactly one, which
causes a sec-critical arbitrary code execution on Firefox stable.

It's worth pointing out that if we only remove the implicit conversion
to T* for rvalues, you still won't be able to directly pass the
returned value to a function that wants T*, even though this is
perfectly safe (because the destructor for the temporary won't be
called until the function call returns, IIUC).  So we still want a new
type for function parameters that accepts implicit conversions from
nsRefPtr/nsCOMPtr, to use instead of raw pointers.  But you can't pass
an already_AddRefed directly to such a function right now anyway, so
this isn't actually a disadvantage relative to the status quo.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: mozilla::TemporaryRef is gone; please use already_AddRefed

[Aryeh Gregor]

On Wed, Jul 1, 2015 at 6:24 AM, Joshua Cranmer  pidgeo...@gmail.com wrote:

 You'd get the same benefit, I think, by making operator T*()  = delete;, 
 syntax which is accepted on gcc 4.8.1+, clang, and MSVC 2015 IIRC.

I once tried this and found it had problematic side effects that made
deploying it difficult in practice.  I think one involved the ternary
operator.  Unfortunately, I can't remember exactly what they were.  :(
 Anyone who's interested can try for themselves by just adding that
line to nsCOMPtr.h, recompiling, and seeing what breaks.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Use of 'auto'

[Aryeh Gregor]

On Wed, Jun 3, 2015 at 12:14 AM, Joshua Cranmer  pidgeo...@gmail.com wrote:
 The case which I am personally very much on the fence is integral types. On
 the one hand, sometimes the type just doesn't matter and you want to make
 sure that you have the same type. On the other hand, I have been very burned
 before by getting the signedness wrong and having code blow up.

Also, what if you make it the wrong type by mistake?  Maybe you
mistyped it, or made an incorrect assumption, or were sloppy in
copy-pasting, or someone changed the type and didn't update all the
callers.  If the type you use is smaller than the real type, you could
wind up truncating.  This is particularly true if it's a signed value
and you incorrectly use an unsigned value.  If I remember correctly,
we have compiler warnings for these errors disabled.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Proposal to alter the coding style to not require the usage of 'virtual' where 'override' is used

[Aryeh Gregor]

On Thu, Apr 30, 2015 at 6:49 PM, Trevor Saunders tbsau...@tbsaunde.org wrote:
 I don't think it would actually be backward incompatible the only
 changes would be turning invalid programs into valid ones.

Given that void foo() override; currently makes foo() a virtual
function, allowing override on non-virtual functions would have to
change the meaning to make foo() non-virtual.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Proposal to alter the coding style to not require the usage of 'virtual' where 'override' is used

[Aryeh Gregor]

On Tue, Apr 28, 2015 at 4:07 AM, Ehsan Akhgari ehsan.akhg...@gmail.com wrote:
 Well, we're not talking about changing C++.  ;-)

My understanding is that the C++ committee will never change the
meaning of existing programs without extremely compelling reason, so
if override currently implies virtual, I think we can safely assume
that will never be changed.

 But why do you find it
 more clear to say |virtual ... final| than |... final|?  They both convey
 the exact same amount of information.  Is it just habit and/or personal
 preference?

Personally, I was surprised when I first learned that override/final
can only be used on virtual functions.  I was definitely unaware that
override/final imply virtual until I read this thread.  It seems David
Baron also didn't know.  So I think keeping virtual makes things
clearer for people who aren't C++ gurus like you.  :)  I would wager
that most of our developers would not realize that void foo()
override; is a virtual function, especially if they didn't think
about it much.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to deprecate: Insecure HTTP

[Aryeh Gregor]

On Tue, Apr 14, 2015 at 3:36 PM, Gervase Markham g...@mozilla.org wrote:
 Yep. That's the system working. CA does something they shouldn't, we
 find out, CA is no longer trusted (perhaps for a time).

 Or do you have an alternative system design where no-one ever makes a
 mistake and all the actors are trustworthy?

No, but it would make sense to require that sites be validated through
a single specific CA, rather than allowing any CA to issue a
certificate for any site.  That would drastically reduce the scope of
attacks: an attacker would have to compromise a single specific CA,
instead of any one of hundreds.  IIRC, HSTS already allows this on an
opt-in basis.  If validation was done via DNSSEC instead of the
existing CA system, this would follow automatically, without sites
having to commit to a single CA.  It also avoids the bootstrapping
problem with HSTS, unless someone has solved that in some other way
and I didn't notice.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: [clipops][editing] document.execCommand('copy', false, 'some data') ?

[Aryeh Gregor]

On Mon, Apr 13, 2015 at 3:18 PM, Hallvord Reiar Michaelsen Steen
hst...@mozilla.com wrote:
 So.. are you suggesting something like

 window.Clipboard.setData('text/plain', 'foo') ?

Maybe.  I don't know what a good name would be.

Re: [clipops][editing] document.execCommand('copy', false, 'some data') ?

[Aryeh Gregor]

On Fri, Apr 10, 2015 at 2:44 PM, Hallvord Reiar Michaelsen Steen
hst...@mozilla.com wrote:
 However, document.execCommand() is spec'ed as having a value argument.
 What about actually using it here? Simplifying the above code to:

 element.onclick = function(){
 document.execCommand('copy', false, 'foo');
 }

Is this really copying?  I think a new function for set clipboard
contents to specified value would make more sense than overloading
execCommand(copy) to mean something more than the standard
text-editor meaning of copy.  Besides, execCommand() is awful and we
should prefer other APIs when possible.

[Bug 584632]

[Aryeh Gregor]

For those who want to see this in Thunderbird 38 -- I suggest talking to
the Thunderbird people and asking them if they can cherry-pick the patch
for Thunderbird without affecting Firefox.  If it's really a huge
improvement for them, maybe they'll be willing to accept it despite lack
of testing.  Perhaps file a bug against the Thunderbird product, or get
on IRC/e-mail/etc. with the appropriate people.

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/584632

Title:
  composer changes font mid email

To manage notifications about this bug go to:
https://bugs.launchpad.net/thunderbird/+bug/584632/+subscriptions

-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

[Desktop-packages] [Bug 584632]

[Aryeh Gregor]

For those who want to see this in Thunderbird 38 -- I suggest talking to
the Thunderbird people and asking them if they can cherry-pick the patch
for Thunderbird without affecting Firefox.  If it's really a huge
improvement for them, maybe they'll be willing to accept it despite lack
of testing.  Perhaps file a bug against the Thunderbird product, or get
on IRC/e-mail/etc. with the appropriate people.

-- 
You received this bug notification because you are a member of Desktop
Packages, which is subscribed to thunderbird in Ubuntu.
https://bugs.launchpad.net/bugs/584632

Title:
  composer changes font mid email

Status in Mozilla Thunderbird Mail and News:
  Confirmed
Status in thunderbird package in Ubuntu:
  Confirmed

Bug description:
  Binary package hint: thunderbird

  As I'm typing my emails in Thunderbird, I can see what appears to be a
  font size change on screen, normally in the second line of text. The
  second line appear smaller than the first. It's barely perceptible, so
  half them time I think I am imagining it.

  Well, I've started Bccing to myself to check, and the emails I am
  receiving from myself are not only a different size, they're also a
  different font. Composer starts in some default serif, and by the
  second line is sans. I'd bee glad to email someone viz thunderbird,
  and also send along a screenshot of how it looks while I am typing.

  Thanks.

To manage notifications about this bug go to:
https://bugs.launchpad.net/thunderbird/+bug/584632/+subscriptions

-- 
Mailing list: https://launchpad.net/~desktop-packages
Post to : desktop-packages@lists.launchpad.net
Unsubscribe : https://launchpad.net/~desktop-packages
More help   : https://help.launchpad.net/ListHelp

[Bug 584632]

[Aryeh Gregor]

(In reply to Jorg K from comment #110)
 Where in the mochitest.ini do I put my new test? I put it right at the front
 since I don't understand the syntax of skip-if. Does that skip the next
 line? Perhaps you can suggest a line where it should go. Or say: After such
 and such.

Just add a line that says
  [test_bug756984.html]
(assuming that's the file's name).  Put it right before the next test line, 
like [test_bug784410.html] or whatever.  The skip-if, support-files, etc. 
lines apply to the preceding test file, so don't put it before one of those 
lines.  (This is confusing.  I looked it up in the online docs: 
https://ci.mozilla.org/job/mozilla-central-docs/Tree_Documentation/build/buildsystem/test_manifests.html)

 In the test I use a span123/span to get offset 3 when clicking behind
 it. Without the span, I get offset 4, which I find surprising, or clicking
 at the front gives offset 1 instead of 0. FF 36 does the same, so I guess
 it's right, but I'd like to understand why 4 and not 3, or 1 and not 0. Why
 does the span make a difference?

You have a newline at the beginning of the div.  That's the first
character in the text node (although it doesn't normally render).  If
you did

  div id='div1'123br456br/div

all on one line, the offsets would be as you expected even without the
span.

 Anyway, the tests are passed. Without my changes, the first test fails as
 expected.
 
 As I said in comment #106: There are already tests for hitting the end key
 and navigating with the arrow keys, so this test should be the only one we
 need additionally.

Sounds great!

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/584632

Title:
  composer changes font mid email

To manage notifications about this bug go to:
https://bugs.launchpad.net/thunderbird/+bug/584632/+subscriptions

-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

[Desktop-packages] [Bug 584632]

[Aryeh Gregor]

(In reply to Jorg K from comment #110)
 Where in the mochitest.ini do I put my new test? I put it right at the front
 since I don't understand the syntax of skip-if. Does that skip the next
 line? Perhaps you can suggest a line where it should go. Or say: After such
 and such.

Just add a line that says
  [test_bug756984.html]
(assuming that's the file's name).  Put it right before the next test line, 
like [test_bug784410.html] or whatever.  The skip-if, support-files, etc. 
lines apply to the preceding test file, so don't put it before one of those 
lines.  (This is confusing.  I looked it up in the online docs: 
https://ci.mozilla.org/job/mozilla-central-docs/Tree_Documentation/build/buildsystem/test_manifests.html)

 In the test I use a span123/span to get offset 3 when clicking behind
 it. Without the span, I get offset 4, which I find surprising, or clicking
 at the front gives offset 1 instead of 0. FF 36 does the same, so I guess
 it's right, but I'd like to understand why 4 and not 3, or 1 and not 0. Why
 does the span make a difference?

You have a newline at the beginning of the div.  That's the first
character in the text node (although it doesn't normally render).  If
you did

  div id='div1'123br456br/div

all on one line, the offsets would be as you expected even without the
span.

 Anyway, the tests are passed. Without my changes, the first test fails as
 expected.
 
 As I said in comment #106: There are already tests for hitting the end key
 and navigating with the arrow keys, so this test should be the only one we
 need additionally.

Sounds great!

-- 
You received this bug notification because you are a member of Desktop
Packages, which is subscribed to thunderbird in Ubuntu.
https://bugs.launchpad.net/bugs/584632

Title:
  composer changes font mid email

Status in Mozilla Thunderbird Mail and News:
  Confirmed
Status in thunderbird package in Ubuntu:
  Confirmed

Bug description:
  Binary package hint: thunderbird

  As I'm typing my emails in Thunderbird, I can see what appears to be a
  font size change on screen, normally in the second line of text. The
  second line appear smaller than the first. It's barely perceptible, so
  half them time I think I am imagining it.

  Well, I've started Bccing to myself to check, and the emails I am
  receiving from myself are not only a different size, they're also a
  different font. Composer starts in some default serif, and by the
  second line is sans. I'd bee glad to email someone viz thunderbird,
  and also send along a screenshot of how it looks while I am typing.

  Thanks.

To manage notifications about this bug go to:
https://bugs.launchpad.net/thunderbird/+bug/584632/+subscriptions

-- 
Mailing list: https://launchpad.net/~desktop-packages
Post to : desktop-packages@lists.launchpad.net
Unsubscribe : https://launchpad.net/~desktop-packages
More help   : https://help.launchpad.net/ListHelp

[Bug 584632]

[Aryeh Gregor]

(In reply to Jorg K from comment #92)
 OK, but how do I send the patch to the try server? I have my level 1
 access rights and I believe SSH is set up correctly. I tried
 hg push -f ssh://mozi...@jorgk.com@hg.mozilla.org/try/ *before* coming
 across the hg qnew command.
 It returned: No changes found or words to that extent. I haven't tried
 after the hg qnew.
 So what is the exact sequence of commands? BTW, I'm on Windows 7. 30 seconds
 of your time save me three hours of (very frustrating) research (since I
 want to focus on the problem and not the infrastructure).

If you have some changes you want to commit, and no patches currently
applied:

  hg qnew mypatchname # This saves your changes to an mq patch
  hg qnew -m try: -b do -p all -u all[x64] -t none try # Make a second patch 
with the try line
  hg push -f mc-try # Make sure you have Magnus' line in .hg/hgrc
  hg qdelete try # Delete the empty try patch

Note: this will leave you with a patch in mq called mypatchname with
your changes.  If you make any new changes to the patch and want to
submit to try again, do the same, except instead of the first line, do
hg qref to refresh the existing patch instead of making a new one.  To
view your existing patches, try hg qser -s; to move around, you can
use hg qpop and hg qpush and hg qgoto; for more help, try hg help
mq or hg help commandname.

If you see any weird changes that only appear on some platforms and don't look 
related to your changes, they're probably random (intermittent) failures that 
are unrelated to your changes.  You can usually spot changes that you really 
caused because they'll show up on all platforms, and look related to your 
changes.  If you're not sure, you can ask us, or ask on IRC for a quicker 
response.
  
 However, this stuff worries me (3x crashed, 1x time out)
 965721 Intermittent test_bug409604.html, test_bug719533.html,
 test_richtext2.html, test_bug412567.html | application crashed [@
 imgStatusTracker::RecordCancel()]
 969526 Intermittent
 test_richtext.html,test_richtext2.html,test_bug436801.html | application
 crashed [@ KERNELBASE.dll + 0x89ae4] (ABORT: Should have mProgressTracker
 until we create mImage: 'mProgressTracker')
 1129538 Intermittent test_draggableprop.html,test_richtext2.html |
 application crashed [@ mozalloc_abort(char const*)] after ABORT: Should
 have given mProgressTracker to mImage: '!mProgressTracker', file
 /image/src/imgRequest.cpp, line 149
 1142900 Intermittent test_richtext2.html | application timed out after 330
 seconds with no output
 
 Any comments on these crashes and time-outs?

Don't worry about those.  These tests fail sometimes at random -- it's
not connected to your changes.

 P.S.: Would you be able to let me know your time-zone so I know when not to
 expect feedback ;-)

I'm UTC+0200, and switching to UTC+0300 this Thursday night.  Last I was
aware, Ehsan lives in eastern Canada and so should be UTC-0400.

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/584632

Title:
  composer changes font mid email

To manage notifications about this bug go to:
https://bugs.launchpad.net/thunderbird/+bug/584632/+subscriptions

-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

[Bug 584632]

[Aryeh Gregor]

(In reply to Jorg K from comment #106)
 Pushed to try server (thanks guys for the support!):
 https://treeherder.mozilla.org/#/jobs?repo=tryrevision=9782fa678cd1

Treeherder isn't loading for me right now, but someone else who looked
said it seemed fine.

 Just out of interest: Why build on all platforms and then just execute on
 Linux? I mean, why build it in the first place? Just to see that it
 compiles? If it already compiled once, and I only rerun tests, that doesn't
 make too much sense.

Yes, to see that it compiles.  Different platforms use different
compilers, and maybe sometimes different compiler versions, and they
treat different things as errors.  Compiling the code on all platforms
is usually a good resources-safety tradeoff.  (Ideally all tests would
be run on all platforms, but it overloads the try servers.)

 Further steps: If this try run goes well, the only thing missing is a test
 for the changes I made. Two scenarios are already covered by the existing
 tests which I had to change: the moving around with the left arrow and the
 jumping to the end of the line with a key stroke. What's missing is the
 clicking beyond the end of the line. The first try run in comment #71 was
 done with only the code to fix the clicking. No tests failed, so there
 wasn't a test for this, so it needs to be added now.

Sounds good!

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/584632

Title:
  composer changes font mid email

To manage notifications about this bug go to:
https://bugs.launchpad.net/thunderbird/+bug/584632/+subscriptions

-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

[Bug 584632]

[Aryeh Gregor]

Try running the tests locally without your patches if you want to be
sure (hg qpop -a will get rid of them if you're using mq).  If they fail
even without your patches, don't worry about it.  In theory all tests
should work on all supported platforms and configurations, but in
practice some small fraction will break on some systems for various
reasons, and in practice we only require that they work on the try
servers.

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/584632

Title:
  composer changes font mid email

To manage notifications about this bug go to:
https://bugs.launchpad.net/thunderbird/+bug/584632/+subscriptions

-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

[Desktop-packages] [Bug 584632]

[Aryeh Gregor]

(In reply to Jorg K from comment #106)
 Pushed to try server (thanks guys for the support!):
 https://treeherder.mozilla.org/#/jobs?repo=tryrevision=9782fa678cd1

Treeherder isn't loading for me right now, but someone else who looked
said it seemed fine.

 Just out of interest: Why build on all platforms and then just execute on
 Linux? I mean, why build it in the first place? Just to see that it
 compiles? If it already compiled once, and I only rerun tests, that doesn't
 make too much sense.

Yes, to see that it compiles.  Different platforms use different
compilers, and maybe sometimes different compiler versions, and they
treat different things as errors.  Compiling the code on all platforms
is usually a good resources-safety tradeoff.  (Ideally all tests would
be run on all platforms, but it overloads the try servers.)

 Further steps: If this try run goes well, the only thing missing is a test
 for the changes I made. Two scenarios are already covered by the existing
 tests which I had to change: the moving around with the left arrow and the
 jumping to the end of the line with a key stroke. What's missing is the
 clicking beyond the end of the line. The first try run in comment #71 was
 done with only the code to fix the clicking. No tests failed, so there
 wasn't a test for this, so it needs to be added now.

Sounds good!

-- 
You received this bug notification because you are a member of Desktop
Packages, which is subscribed to thunderbird in Ubuntu.
https://bugs.launchpad.net/bugs/584632

Title:
  composer changes font mid email

Status in Mozilla Thunderbird Mail and News:
  Confirmed
Status in thunderbird package in Ubuntu:
  Confirmed

Bug description:
  Binary package hint: thunderbird

  As I'm typing my emails in Thunderbird, I can see what appears to be a
  font size change on screen, normally in the second line of text. The
  second line appear smaller than the first. It's barely perceptible, so
  half them time I think I am imagining it.

  Well, I've started Bccing to myself to check, and the emails I am
  receiving from myself are not only a different size, they're also a
  different font. Composer starts in some default serif, and by the
  second line is sans. I'd bee glad to email someone viz thunderbird,
  and also send along a screenshot of how it looks while I am typing.

  Thanks.

To manage notifications about this bug go to:
https://bugs.launchpad.net/thunderbird/+bug/584632/+subscriptions

-- 
Mailing list: https://launchpad.net/~desktop-packages
Post to : desktop-packages@lists.launchpad.net
Unsubscribe : https://launchpad.net/~desktop-packages
More help   : https://help.launchpad.net/ListHelp

[Desktop-packages] [Bug 584632]

[Aryeh Gregor]

Try running the tests locally without your patches if you want to be
sure (hg qpop -a will get rid of them if you're using mq).  If they fail
even without your patches, don't worry about it.  In theory all tests
should work on all supported platforms and configurations, but in
practice some small fraction will break on some systems for various
reasons, and in practice we only require that they work on the try
servers.

-- 
You received this bug notification because you are a member of Desktop
Packages, which is subscribed to thunderbird in Ubuntu.
https://bugs.launchpad.net/bugs/584632

Title:
  composer changes font mid email

Status in Mozilla Thunderbird Mail and News:
  Confirmed
Status in thunderbird package in Ubuntu:
  Confirmed

Bug description:
  Binary package hint: thunderbird

  As I'm typing my emails in Thunderbird, I can see what appears to be a
  font size change on screen, normally in the second line of text. The
  second line appear smaller than the first. It's barely perceptible, so
  half them time I think I am imagining it.

  Well, I've started Bccing to myself to check, and the emails I am
  receiving from myself are not only a different size, they're also a
  different font. Composer starts in some default serif, and by the
  second line is sans. I'd bee glad to email someone viz thunderbird,
  and also send along a screenshot of how it looks while I am typing.

  Thanks.

To manage notifications about this bug go to:
https://bugs.launchpad.net/thunderbird/+bug/584632/+subscriptions

-- 
Mailing list: https://launchpad.net/~desktop-packages
Post to : desktop-packages@lists.launchpad.net
Unsubscribe : https://launchpad.net/~desktop-packages
More help   : https://help.launchpad.net/ListHelp

[Desktop-packages] [Bug 584632]

[Aryeh Gregor]

(In reply to Jorg K from comment #92)
 OK, but how do I send the patch to the try server? I have my level 1
 access rights and I believe SSH is set up correctly. I tried
 hg push -f ssh://mozi...@jorgk.com@hg.mozilla.org/try/ *before* coming
 across the hg qnew command.
 It returned: No changes found or words to that extent. I haven't tried
 after the hg qnew.
 So what is the exact sequence of commands? BTW, I'm on Windows 7. 30 seconds
 of your time save me three hours of (very frustrating) research (since I
 want to focus on the problem and not the infrastructure).

If you have some changes you want to commit, and no patches currently
applied:

  hg qnew mypatchname # This saves your changes to an mq patch
  hg qnew -m try: -b do -p all -u all[x64] -t none try # Make a second patch 
with the try line
  hg push -f mc-try # Make sure you have Magnus' line in .hg/hgrc
  hg qdelete try # Delete the empty try patch

Note: this will leave you with a patch in mq called mypatchname with
your changes.  If you make any new changes to the patch and want to
submit to try again, do the same, except instead of the first line, do
hg qref to refresh the existing patch instead of making a new one.  To
view your existing patches, try hg qser -s; to move around, you can
use hg qpop and hg qpush and hg qgoto; for more help, try hg help
mq or hg help commandname.

If you see any weird changes that only appear on some platforms and don't look 
related to your changes, they're probably random (intermittent) failures that 
are unrelated to your changes.  You can usually spot changes that you really 
caused because they'll show up on all platforms, and look related to your 
changes.  If you're not sure, you can ask us, or ask on IRC for a quicker 
response.
  
 However, this stuff worries me (3x crashed, 1x time out)
 965721 Intermittent test_bug409604.html, test_bug719533.html,
 test_richtext2.html, test_bug412567.html | application crashed [@
 imgStatusTracker::RecordCancel()]
 969526 Intermittent
 test_richtext.html,test_richtext2.html,test_bug436801.html | application
 crashed [@ KERNELBASE.dll + 0x89ae4] (ABORT: Should have mProgressTracker
 until we create mImage: 'mProgressTracker')
 1129538 Intermittent test_draggableprop.html,test_richtext2.html |
 application crashed [@ mozalloc_abort(char const*)] after ABORT: Should
 have given mProgressTracker to mImage: '!mProgressTracker', file
 /image/src/imgRequest.cpp, line 149
 1142900 Intermittent test_richtext2.html | application timed out after 330
 seconds with no output
 
 Any comments on these crashes and time-outs?

Don't worry about those.  These tests fail sometimes at random -- it's
not connected to your changes.

 P.S.: Would you be able to let me know your time-zone so I know when not to
 expect feedback ;-)

I'm UTC+0200, and switching to UTC+0300 this Thursday night.  Last I was
aware, Ehsan lives in eastern Canada and so should be UTC-0400.

-- 
You received this bug notification because you are a member of Desktop
Packages, which is subscribed to thunderbird in Ubuntu.
https://bugs.launchpad.net/bugs/584632

Title:
  composer changes font mid email

Status in Mozilla Thunderbird Mail and News:
  Confirmed
Status in thunderbird package in Ubuntu:
  Confirmed

Bug description:
  Binary package hint: thunderbird

  As I'm typing my emails in Thunderbird, I can see what appears to be a
  font size change on screen, normally in the second line of text. The
  second line appear smaller than the first. It's barely perceptible, so
  half them time I think I am imagining it.

  Well, I've started Bccing to myself to check, and the emails I am
  receiving from myself are not only a different size, they're also a
  different font. Composer starts in some default serif, and by the
  second line is sans. I'd bee glad to email someone viz thunderbird,
  and also send along a screenshot of how it looks while I am typing.

  Thanks.

To manage notifications about this bug go to:
https://bugs.launchpad.net/thunderbird/+bug/584632/+subscriptions

-- 
Mailing list: https://launchpad.net/~desktop-packages
Post to : desktop-packages@lists.launchpad.net
Unsubscribe : https://launchpad.net/~desktop-packages
More help   : https://help.launchpad.net/ListHelp

[Bug 584632]

[Aryeh Gregor]

Okay, this caused some try failures.  One of them is an unexpected pass in 
richtext2, which is good!  It means you just have to update the test so it 
knows we're supposed to pass now.  In the case of richtext2, you want to edit 
the file editor/libeditor/tests/browserscope/lib/richtext2/currentStatus.js and 
remove the appropriate lines (this is different from how most tests need to be 
updated).  To check that it worked, run this (which may take a while and you 
have to keep the browser window focused):
  ./mach mochitest-plain editor/libeditor/tests/browserscope
If you're on Linux, you should be able to install the appropriate package for 
the xvfb-run command and use this instead so it runs without creating a window 
you have to keep in focus:
  xvfb-run ./mach mochitest-plain editor/libeditor/tests/browserscope

The other failure I see is
layout/generic/test/test_movement_by_characters.html, which you need to
look at.  It could be your patch has a problem, or it could be the test
needs to be updated.  If you're having trouble, please feel free to ask
for help!

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/584632

Title:
  composer changes font mid email

To manage notifications about this bug go to:
https://bugs.launchpad.net/thunderbird/+bug/584632/+subscriptions

-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

[Bug 584632]

[Aryeh Gregor]

A good try line to use by default for editor changes is:
  try: -b do -p all -u all[x64] -t none
This will build on all platforms (to detect compile errors), and will run tests 
only on 64-bit Linux (to avoid wasting resources).  If there might be 
platform-specific test failures, remove the [x64], but I don't think you need 
to do that here.

For Mercurial -- if you're using mq (which you should), you don't ever
want to do hg commit.  The patch you've attached is exactly right.

Here are the try results for you:
https://treeherder.mozilla.org/#/jobs?repo=tryrevision=fd11f71e3daa

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/584632

Title:
  composer changes font mid email

To manage notifications about this bug go to:
https://bugs.launchpad.net/thunderbird/+bug/584632/+subscriptions

-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

[Desktop-packages] [Bug 584632]

[Aryeh Gregor]

A good try line to use by default for editor changes is:
  try: -b do -p all -u all[x64] -t none
This will build on all platforms (to detect compile errors), and will run tests 
only on 64-bit Linux (to avoid wasting resources).  If there might be 
platform-specific test failures, remove the [x64], but I don't think you need 
to do that here.

For Mercurial -- if you're using mq (which you should), you don't ever
want to do hg commit.  The patch you've attached is exactly right.

Here are the try results for you:
https://treeherder.mozilla.org/#/jobs?repo=tryrevision=fd11f71e3daa

-- 
You received this bug notification because you are a member of Desktop
Packages, which is subscribed to thunderbird in Ubuntu.
https://bugs.launchpad.net/bugs/584632

Title:
  composer changes font mid email

Status in Mozilla Thunderbird Mail and News:
  Confirmed
Status in thunderbird package in Ubuntu:
  Confirmed

Bug description:
  Binary package hint: thunderbird

  As I'm typing my emails in Thunderbird, I can see what appears to be a
  font size change on screen, normally in the second line of text. The
  second line appear smaller than the first. It's barely perceptible, so
  half them time I think I am imagining it.

  Well, I've started Bccing to myself to check, and the emails I am
  receiving from myself are not only a different size, they're also a
  different font. Composer starts in some default serif, and by the
  second line is sans. I'd bee glad to email someone viz thunderbird,
  and also send along a screenshot of how it looks while I am typing.

  Thanks.

To manage notifications about this bug go to:
https://bugs.launchpad.net/thunderbird/+bug/584632/+subscriptions

-- 
Mailing list: https://launchpad.net/~desktop-packages
Post to : desktop-packages@lists.launchpad.net
Unsubscribe : https://launchpad.net/~desktop-packages
More help   : https://help.launchpad.net/ListHelp

[Desktop-packages] [Bug 584632]

[Aryeh Gregor]

Okay, this caused some try failures.  One of them is an unexpected pass in 
richtext2, which is good!  It means you just have to update the test so it 
knows we're supposed to pass now.  In the case of richtext2, you want to edit 
the file editor/libeditor/tests/browserscope/lib/richtext2/currentStatus.js and 
remove the appropriate lines (this is different from how most tests need to be 
updated).  To check that it worked, run this (which may take a while and you 
have to keep the browser window focused):
  ./mach mochitest-plain editor/libeditor/tests/browserscope
If you're on Linux, you should be able to install the appropriate package for 
the xvfb-run command and use this instead so it runs without creating a window 
you have to keep in focus:
  xvfb-run ./mach mochitest-plain editor/libeditor/tests/browserscope

The other failure I see is
layout/generic/test/test_movement_by_characters.html, which you need to
look at.  It could be your patch has a problem, or it could be the test
needs to be updated.  If you're having trouble, please feel free to ask
for help!

-- 
You received this bug notification because you are a member of Desktop
Packages, which is subscribed to thunderbird in Ubuntu.
https://bugs.launchpad.net/bugs/584632

Title:
  composer changes font mid email

Status in Mozilla Thunderbird Mail and News:
  Confirmed
Status in thunderbird package in Ubuntu:
  Confirmed

Bug description:
  Binary package hint: thunderbird

  As I'm typing my emails in Thunderbird, I can see what appears to be a
  font size change on screen, normally in the second line of text. The
  second line appear smaller than the first. It's barely perceptible, so
  half them time I think I am imagining it.

  Well, I've started Bccing to myself to check, and the emails I am
  receiving from myself are not only a different size, they're also a
  different font. Composer starts in some default serif, and by the
  second line is sans. I'd bee glad to email someone viz thunderbird,
  and also send along a screenshot of how it looks while I am typing.

  Thanks.

To manage notifications about this bug go to:
https://bugs.launchpad.net/thunderbird/+bug/584632/+subscriptions

-- 
Mailing list: https://launchpad.net/~desktop-packages
Post to : desktop-packages@lists.launchpad.net
Unsubscribe : https://launchpad.net/~desktop-packages
More help   : https://help.launchpad.net/ListHelp

[Desktop-packages] [Bug 584632]

[Aryeh Gregor]

The failure in editor/libeditor/tests/test_selection_move_commands.xul
also needs looking at.  Otherwise, looks good!

-- 
You received this bug notification because you are a member of Desktop
Packages, which is subscribed to thunderbird in Ubuntu.
https://bugs.launchpad.net/bugs/584632

Title:
  composer changes font mid email

Status in Mozilla Thunderbird Mail and News:
  Confirmed
Status in thunderbird package in Ubuntu:
  Confirmed

Bug description:
  Binary package hint: thunderbird

  As I'm typing my emails in Thunderbird, I can see what appears to be a
  font size change on screen, normally in the second line of text. The
  second line appear smaller than the first. It's barely perceptible, so
  half them time I think I am imagining it.

  Well, I've started Bccing to myself to check, and the emails I am
  receiving from myself are not only a different size, they're also a
  different font. Composer starts in some default serif, and by the
  second line is sans. I'd bee glad to email someone viz thunderbird,
  and also send along a screenshot of how it looks while I am typing.

  Thanks.

To manage notifications about this bug go to:
https://bugs.launchpad.net/thunderbird/+bug/584632/+subscriptions

-- 
Mailing list: https://launchpad.net/~desktop-packages
Post to : desktop-packages@lists.launchpad.net
Unsubscribe : https://launchpad.net/~desktop-packages
More help   : https://help.launchpad.net/ListHelp

[Bug 584632]

[Aryeh Gregor]

The failure in editor/libeditor/tests/test_selection_move_commands.xul
also needs looking at.  Otherwise, looks good!

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/584632

Title:
  composer changes font mid email

To manage notifications about this bug go to:
https://bugs.launchpad.net/thunderbird/+bug/584632/+subscriptions

-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

[Desktop-packages] [Bug 584632]

[Aryeh Gregor]

For the record, from black-box testing of WebKit a few years ago, it
looked like it normalized the selection after every change.  Even if you
called .addRange(), it copied the range and then stuck the selection
endpoints inside a nearby text node if available, etc.  I think it's
taking things too far to change script-specified selections, but the
right way to do this is probably to have some sort of helper method in
Selection like NormalizePoint(nsINode*, int32_t) and call it before
every user-initiated selection change.  We might want to disallow other
types of user-created selections from occurring in the future, although
my brain is too rusty to supply any.

Do we want to allow a selection like bfoo/b{}ibar/i, with the
selection collapsed in between the b and i?  IIRC, WebKit in my
testing forced this to be bfoo[]/bibar/i (always making it on
the previous text node).  A ten-second test in WordPad suggests this is
the right thing to do.

I don't think any of this has to be in the scope of the current bug,
though.

-- 
You received this bug notification because you are a member of Desktop
Packages, which is subscribed to thunderbird in Ubuntu.
https://bugs.launchpad.net/bugs/584632

Title:
  composer changes font mid email

Status in Mozilla Thunderbird Mail and News:
  Confirmed
Status in thunderbird package in Ubuntu:
  Confirmed

Bug description:
  Binary package hint: thunderbird

  As I'm typing my emails in Thunderbird, I can see what appears to be a
  font size change on screen, normally in the second line of text. The
  second line appear smaller than the first. It's barely perceptible, so
  half them time I think I am imagining it.

  Well, I've started Bccing to myself to check, and the emails I am
  receiving from myself are not only a different size, they're also a
  different font. Composer starts in some default serif, and by the
  second line is sans. I'd bee glad to email someone viz thunderbird,
  and also send along a screenshot of how it looks while I am typing.

  Thanks.

To manage notifications about this bug go to:
https://bugs.launchpad.net/thunderbird/+bug/584632/+subscriptions

-- 
Mailing list: https://launchpad.net/~desktop-packages
Post to : desktop-packages@lists.launchpad.net
Unsubscribe : https://launchpad.net/~desktop-packages
More help   : https://help.launchpad.net/ListHelp

[Bug 584632]

[Aryeh Gregor]

For the record, from black-box testing of WebKit a few years ago, it
looked like it normalized the selection after every change.  Even if you
called .addRange(), it copied the range and then stuck the selection
endpoints inside a nearby text node if available, etc.  I think it's
taking things too far to change script-specified selections, but the
right way to do this is probably to have some sort of helper method in
Selection like NormalizePoint(nsINode*, int32_t) and call it before
every user-initiated selection change.  We might want to disallow other
types of user-created selections from occurring in the future, although
my brain is too rusty to supply any.

Do we want to allow a selection like bfoo/b{}ibar/i, with the
selection collapsed in between the b and i?  IIRC, WebKit in my
testing forced this to be bfoo[]/bibar/i (always making it on
the previous text node).  A ten-second test in WordPad suggests this is
the right thing to do.

I don't think any of this has to be in the scope of the current bug,
though.

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/584632

Title:
  composer changes font mid email

To manage notifications about this bug go to:
https://bugs.launchpad.net/thunderbird/+bug/584632/+subscriptions

-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

[Desktop-packages] [Bug 584632]

[Aryeh Gregor]

The try results look good to me, so you just need to include an
automated regression test (mochitest) and you can ask a reviewer to
approve it to be included in Firefox.  You want to add a file patterned
off something like this: https://dxr.mozilla.org/mozilla-
central/source/layout/generic/test/test_bug970363.html  You could base
it off one of the tests you attached to this bug, but instead of asking
the user to click, you have to synthesize a click using a function from
EventUtils.js https://dxr.mozilla.org/mozilla-
central/source/testing/mochitest/tests/SimpleTest/EventUtils.js,
possibly synthesizeMouse.  And instead of doing alert(), do something
like is(myVariable, #text, selection should be in text node).  You
have to add your test's name to the file
layout/generic/test/mochitest.ini, and then you can run it with ./mach
mochitest-plain layout/generic/test/test_bug756984.html.  It should
fail before the patch is applied, and pass after the patch is applied.

Thanks a ton for working on this!  If you want feedback from me on if
your test looks good before asking for review, please feel free to ask
me via the feedback flag when uploading your patch.  I'm probably less
busy than the reviewers.  :)

-- 
You received this bug notification because you are a member of Desktop
Packages, which is subscribed to thunderbird in Ubuntu.
https://bugs.launchpad.net/bugs/584632

Title:
  composer changes font mid email

Status in Mozilla Thunderbird Mail and News:
  Confirmed
Status in thunderbird package in Ubuntu:
  Confirmed

Bug description:
  Binary package hint: thunderbird

  As I'm typing my emails in Thunderbird, I can see what appears to be a
  font size change on screen, normally in the second line of text. The
  second line appear smaller than the first. It's barely perceptible, so
  half them time I think I am imagining it.

  Well, I've started Bccing to myself to check, and the emails I am
  receiving from myself are not only a different size, they're also a
  different font. Composer starts in some default serif, and by the
  second line is sans. I'd bee glad to email someone viz thunderbird,
  and also send along a screenshot of how it looks while I am typing.

  Thanks.

To manage notifications about this bug go to:
https://bugs.launchpad.net/thunderbird/+bug/584632/+subscriptions

-- 
Mailing list: https://launchpad.net/~desktop-packages
Post to : desktop-packages@lists.launchpad.net
Unsubscribe : https://launchpad.net/~desktop-packages
More help   : https://help.launchpad.net/ListHelp

[Desktop-packages] [Bug 584632]

[Aryeh Gregor]

Oops, I missed that part of comment #68 -- should have read more
carefully.  I don't know what more there is to do, since it passed a try
run.  But that's why Ehsan is in charge and not me.  ;)  In any event,
you will need a test at the end of the day, so you could still go ahead
and write it now.

-- 
You received this bug notification because you are a member of Desktop
Packages, which is subscribed to thunderbird in Ubuntu.
https://bugs.launchpad.net/bugs/584632

Title:
  composer changes font mid email

Status in Mozilla Thunderbird Mail and News:
  Confirmed
Status in thunderbird package in Ubuntu:
  Confirmed

Bug description:
  Binary package hint: thunderbird

  As I'm typing my emails in Thunderbird, I can see what appears to be a
  font size change on screen, normally in the second line of text. The
  second line appear smaller than the first. It's barely perceptible, so
  half them time I think I am imagining it.

  Well, I've started Bccing to myself to check, and the emails I am
  receiving from myself are not only a different size, they're also a
  different font. Composer starts in some default serif, and by the
  second line is sans. I'd bee glad to email someone viz thunderbird,
  and also send along a screenshot of how it looks while I am typing.

  Thanks.

To manage notifications about this bug go to:
https://bugs.launchpad.net/thunderbird/+bug/584632/+subscriptions

-- 
Mailing list: https://launchpad.net/~desktop-packages
Post to : desktop-packages@lists.launchpad.net
Unsubscribe : https://launchpad.net/~desktop-packages
More help   : https://help.launchpad.net/ListHelp

[Bug 584632]

[Aryeh Gregor]

Oops, I missed that part of comment #68 -- should have read more
carefully.  I don't know what more there is to do, since it passed a try
run.  But that's why Ehsan is in charge and not me.  ;)  In any event,
you will need a test at the end of the day, so you could still go ahead
and write it now.

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/584632

Title:
  composer changes font mid email

To manage notifications about this bug go to:
https://bugs.launchpad.net/thunderbird/+bug/584632/+subscriptions

-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

[Bug 584632]

[Aryeh Gregor]

The try results look good to me, so you just need to include an
automated regression test (mochitest) and you can ask a reviewer to
approve it to be included in Firefox.  You want to add a file patterned
off something like this: https://dxr.mozilla.org/mozilla-
central/source/layout/generic/test/test_bug970363.html  You could base
it off one of the tests you attached to this bug, but instead of asking
the user to click, you have to synthesize a click using a function from
EventUtils.js https://dxr.mozilla.org/mozilla-
central/source/testing/mochitest/tests/SimpleTest/EventUtils.js,
possibly synthesizeMouse.  And instead of doing alert(), do something
like is(myVariable, #text, selection should be in text node).  You
have to add your test's name to the file
layout/generic/test/mochitest.ini, and then you can run it with ./mach
mochitest-plain layout/generic/test/test_bug756984.html.  It should
fail before the patch is applied, and pass after the patch is applied.

Thanks a ton for working on this!  If you want feedback from me on if
your test looks good before asking for review, please feel free to ask
me via the feedback flag when uploading your patch.  I'm probably less
busy than the reviewers.  :)

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/584632

Title:
  composer changes font mid email

To manage notifications about this bug go to:
https://bugs.launchpad.net/thunderbird/+bug/584632/+subscriptions

-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Re: Intent to deprecate: persistent permissions over HTTP

[Aryeh Gregor]

On Mon, Mar 16, 2015 at 3:24 PM, Eric Rescorla e...@rtfm.com wrote:
 Lots of people have the cameras in their rooms pointing at them even when
 they are not using the computer, and so the camera can be used to spy on
 them (Again, I refer you to Checkoway's description of ratting [1]). This
 might be more obvious if you think about the microphone. I assume you can
 see the value of my remotely accessing the microphone on your phone even
 when you are not actively using it?

Yes, that makes it perfectly clear.  Thank you.

 They already have to be HTTPS. The background for this discussion is that
 getUserMedia() enforces the policy that Anne is proposing.

I was unaware of that.  That sounds very reasonable.

 I'm really confused by what you are arguing for, since the text that you
 quote is a response to you writing

 Why isn't the user prompted before every picture is taken?  Is there
 really a use-case for allowing a site to take pictures without the
 user's case-by-case permission that outweighs the privacy issues?

 So I took from this that you wanted a consent prompt every time.

I want that for getUserMedia, yes.  It scares me that even HTTPS sites
are allowed to persist this permission, because server-side
compromises are common.  But if we have to allow it at all, it makes
sense to limit the attack surface as much as possible, even if I'm not
sure about how effective this measure is in preventing attacks in
practice.

 What Anne is proposing (and I support) is that the browser be allowed to
 persist consent only on HTTPS sites (the details of when it would do
 so vary between APIs and between browsers, perhaps). This is the current
 state of play for getUserMedia (camera and microphone) but not for other
 APIs. How is it you believe that the browser should behave?

I think this makes sense for getUserMedia, at a minimum.  I think
other APIs need to be considered on a case-by-case basis, because this
doesn't fully solve the security problem, annoys the user, and causes
permissions-prompt fatigue.  I don't think a blanket policy of only
persisting any permissions over HTTPS is a good idea, e.g., for
pop-ups.  So I think I actually more or less agree with most people
here after all.  :)
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to deprecate: persistent permissions over HTTP

[Aryeh Gregor]

On Thu, Mar 12, 2015 at 3:56 PM, Adam Roach a...@mozilla.com wrote:
 As an aside, the first three are not just fixable, but actually fixed within
 the next few months: https://letsencrypt.org/

That seems like a huge step forward.  But putting my ex-sysadmin hat
on -- assuming it works as advertised, there are still significant
remaining practical issues for at least some sites:

1) SNI is reportedly still not usable if you care about IE on XP.
This means HTTPS is not usable on shared hosting, which is most small
sites, unless you don't care that your site doesn't load in IE on XP.
This is also a problem for larger sites whose content is accessible
via multiple domains (even just www.foo.com vs. foo.com), unless they
want to get an IP address per domain.  For instance, Wikipedia serves
a whole bunch of second-level domains (wikipedia.org, wikimedia.org,
wiktionary.org, etc.) from the same servers, and to support HTTPS,
they needed to reconfigure their site so that all of these were
different IP addresses.

2) If you want to support access via both HTTP and HTTPS for whatever
reason, you have to make sure your content uses protocol-relative URLs
exclusively, which means making modifying the software that runs on
your site.  Otherwise users will click a link and get sent back to the
insecure site without noticing.  This could include user-provided
URLs.  You could just use HTTPS exclusively, but that's a somewhat
bigger step to take.

3) If you include third-party scripts that are not available over
HTTPS, at least Chrome will helpfully break your site until your users
click through a permissions dialog, if I remember correctly.

4) According to the O'Reilly book linked from istlsfastyet.com,
best-case TLS usage still adds a round-trip to every connection.
Common non-best-case scenarios are worse (e.g., IE  10 apparently
doesn't support False Start).  This is a nontrivial performance
penalty.

There are probably other practical issues that will crop up on
specific sites.  It's still a change that requires nonzero effort to
test and deploy, and has downsides, so using HTTPS is not necessarily
a no-brainer for everyone.  For instance, Wikimedia took a long time
to deploy HTTPS, even though setting up certificates was not an issue,
because of the various side issues that had to be handled (at least 1
and 2).  So use HTTPS isn't an easy solution for everyone.  But yes,
this project should remove one of the big issues for a lot of people.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to deprecate: persistent permissions over HTTP

[Aryeh Gregor]

On Thu, Mar 12, 2015 at 9:42 PM, Boris Zbarsky bzbar...@mit.edu wrote:
 On 3/12/15 3:31 PM, Aryeh Gregor wrote:

 2) Attacker opens a background tab and navigates it to http://b.com (I
 can't think of a JavaScript way to do this, but if there isn't one,
 making a big a href=b.com target=_blank that covers the whole page
 would work well enough)

 This is presuming user interaction.  I agree that attacks that rely on user
 interaction are also a problem here, but I'm _really_ scared by the
 potential of no-interaction needed attacks, which can happen when the user
 is not even actively using the computer.  Maybe it's just me.

What's the use of taking a picture if the user isn't actively using
the computer?  Also, the user will almost certainly return to the
computer at some point, and the attacker can probably wait till then.

On Thu, Mar 12, 2015 at 10:53 PM, Eric Rescorla e...@rtfm.com wrote:
 Yes. User consent failure represents a large fraction of failures on
 video conferencing sites.

Hmm.  I guess I'm not qualified to say whether this is worth it, but
it still does scare me.  Would these sites care if they have to be
HTTPS?

 Also, continually prompting users for
 permissions weakens protections against users granting consent
 to malicious sites.

 See also Adam Barth's
 Prompting the User Is a Security Failure at
 http://rtc-web.alvestrand.com/home/papers

Thoroughly agreed, and that is exactly what this proposal would do --
make users click through lots of extra permissions dialogs.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to deprecate: persistent permissions over HTTP

[Aryeh Gregor]

On Thu, Mar 12, 2015 at 4:34 PM, Ehsan Akhgari ehsan.akhg...@gmail.com wrote:
 2) If the only common real-world MITM threat is via a compromise
 adjacent to the client (e.g., wireless), there's no reason to restrict
 geolocation, because the attacker already knows the user's location
 fairly precisely.


 I don't think that is the only common real-world attack.  Other types
 include your traffic being intercepted by your ISP, and/or your government.

I guess it's hard to say how common those are in practice, or how much
of a concern they are.  I agree that for an API that allows taking
pictures without the user's case-by-case permission, it would pay to
err far on the safe side.

I'm actually rather disturbed that such an API even exists.  Even if
the site is HTTPS, it could be hacked, or it could be spoofed, or the
operators could just be abusive.  Every site must be assumed possibly
malicious, no matter how many permissions dialogs the user clicks
through, and HTTPS can be assumed to be only modestly safer than HTTP.
Why isn't the user prompted before every picture is taken?  Is there
really a use-case for allowing a site to take pictures without the
user's case-by-case permission that outweighs the privacy issues?

As for geolocation, I'm still not convinced that it's worth worrying
about here.  The ISP and government probably have better ways of
tracking down the user's location.  The ISP generally knows where the
Internet connection goes regardless, and the government can probably
get the info from the ISP (after all, it was able to install a MITM).

 There have been documented cases of webcam spying victims committing
 suicide.  And I wouldn't be surprised if there are or will be businesses
 based on selling people's webcam feeds.  Protecting people's physical
 privacy is just as important as protecting their digital privacy.

Then why only focus on attacks that are foiled by HTTPS?  We should be
equally concerned with attacks that HTTPS doesn't prevent, which I
think are probably much more common.

On Thu, Mar 12, 2015 at 5:24 PM, Boris Zbarsky bzbar...@mit.edu wrote:
 The attack scenario I'm thinking is:

 1) User loads http://a.com
 2) Attacker immediately sets location to http://b.com
 3) Attacker's hacked-up b.com goes fullscreen, pretending to still be a.com
 to the user by spoofing browser chrome, while also turning on the camera
 because the user granted permission to b.com to do that at some point.

How about:

1) User loads http://a.com
2) Attacker opens a background tab and navigates it to http://b.com (I
can't think of a JavaScript way to do this, but if there isn't one,
making a big a href=b.com target=_blank that covers the whole page
would work well enough)
3) http://b.com loads in 10 ms because it's really being served by the
MITM, uses the permission, and closes itself
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to deprecate: persistent permissions over HTTP

[Aryeh Gregor]

On Tue, Mar 10, 2015 at 5:00 PM, Boris Zbarsky bzbar...@mit.edu wrote:
 The mitigation applies in this situation:

 1)  User connects to a MITMed network (e.g. wireless at the airport or
 coffeeshop or whatever) which I will henceforth call the attacker.
 2)  No matter what site the user loads, the attacker injects a hidden
 iframe claiming to be from hostname X that the user has granted a
 persistent permissions grant to.
 3)  The attacker now turns the camera/microphone/whatever.

Aha, that makes a lot more sense.  Thanks.  Yes, that does seem like a
more realistic attack.  A few points come to mind:

1) The page has no way to know whether it has persisted permissions
without just trying, right?  If so, the user will notice something is
weird when he gets strange permissions requests, which makes the
attack less attractive.

2) If the only common real-world MITM threat is via a compromise
adjacent to the client (e.g., wireless), there's no reason to restrict
geolocation, because the attacker already knows the user's location
fairly precisely.

3) Is there any reason to not persist permissions for as long as the
user remains on the same network (assuming we can figure that out
reliably)?  If not, the proposal would be much less annoying, because
in many common cases the permission would be persisted for a long time
anyway.  Better yet, can we ask the OS whether the network is
classified as home/work/public and only restrict the persistence for
public networks?

4) Feasible though the attack may be, I'm not sure how likely
attackers are to try it.  Is there some plausible profit motive here?
Script kiddies will set up websites and portscan with botnets just for
lulz, but a malicious wireless router requires physical presence,
which is much riskier for the attacker.  If I compromised a public
wireless router, I would try passively sniffing for credit card info
in people's unencrypted webmail, or steal their login info.  Why would
I blow my cover by trying to take pictures of them?

 Right, and only work if the user loads such a site themselves on that
 network.  If I load cnn.com and get a popup asking whether Google Hangouts
 can turn on my camera, I'd get a bit suspicious... (though I bet a lot of
 people would just click through anyway).

Especially because it says Google Hangouts wants the permission.  Why
wouldn't I give permission to Google Hangouts, if I use it regularly?
Maybe it's a bit puzzling that it's asking me right now, but computers
are weird, it probably has some reason.  If it was some site I didn't
recognize I might say no, but not if it's a site I use all the time.

I'm not convinced that the proposal increases real-world security
enough to warrant any reduction at all in user convenience.

 Switch to HTTPS is not a reasonable solution.


 Why not?

Because unless things have changed a lot in the last three years or
so, HTTPS is a pain for a few reasons:

1) It requires time and effort to set up.  Network admins have better
things to do.  Most of them either are volunteers, work part-time,
computers isn't their primary job responsibility, they're overworked,
etc.

2) It adds an additional point of failure.  It's easy to misconfigure,
and you have to keep the certificate up-to-date.  If you mess up,
browsers will helpfully go berserk and tell your users that your site
is trying to hack their computer (or that's what users will infer from
the terrifying bright-red warnings).  This is not a simple problem to
solve -- for a long time, https://amazon.com would give a cert error,
and I'm pretty sure I once saw an error on a Google property too.  I
think Microsoft too once.

3) Last I checked, if you want a cert that works in all browsers, you
need to pay money.  This is a big psychological hurdle for some
people, and may be unreasonable for people who manage a lot of small
domains.

4) It adds round-trips, which is a big deal for people on high-latency
connections.  I remember Google was trying to cut it down to one extra
round-trip on the first connection and none on subsequent connections,
but I don't know if that's actually made it into all the major
browsers yet.

These issues seem all basically fixable within a few years, if the
major stakeholders were on board.  But until they're fixed, there are
good reasons for sysadmins to be reluctant to use SSL.  Ideally,
setting up SSL would like something like this: the webserver
automatically generates a key pair, submits the public key to its
nameserver to be put into its domain's DNSSEC CERT record, queries the
resulting DNSSEC record, and serves it to browsers as its certificate;
and of course automatically re-queries the record periodically so it
doesn't expire.  The nameserver can verify the server's IP address
matches the A record to to ensure that it's the right one, unless
someone has compromised the backbone or the nameserver's local
network.  In theory you don't need DNSSEC, CACert or whatever would
work too.  You would 

Re: Intent to deprecate: persistent permissions over HTTP

[Aryeh Gregor]

On Sat, Mar 7, 2015 at 10:33 PM, Eric Rescorla e...@rtfm.com wrote:
 Let's consider a different example than the one you propose: access
 to the camera and microphone via getUserMedia(). Say that a site
 adds a feature which lets you take a picture of yourself for your
 avatar (come to think of it, I wish github did this). If the permissions
 are persistent, then the site (or if HTTPS isn't used, any network
 attacker) can access my camera and see what's going on in my
 room at any time [0] and largely without my knowledge.
 By contrast, if I need to click OK in order to give a remote site access
 to my camera (even if I generally do consent without much thought)
 this makes the attack much more difficult to mount.

So the mitigation applies when:

1) Some users have already persisted the permission for the site.

2) The site asks for the permission either predictably or infrequently
enough that the user is not conditioned to just click yes every time
anyway.

A limitation on the mitigation is that if the site asks for the
permission during regular use, the attacker could just make sure that
their permissions request appears at that time, and the user would
click yes because they expect the request at that time anyway.
However, this would require the attacker to do some more work, and
would only work some of the time (if the site is expected to ask for
the permission during the MITM'd session).

The disadvantage is that non-secured sites that depend heavily on any
of these permissions would get more annoying to use.  Switch to
HTTPS is not a reasonable solution.  This could be helped by allowing
the user to give permission for the session, but that would also
reduce the effectiveness of the mitigation.

Another point to make is that whenever the site actually requests the
info legitimately (takes a picture, gets geolocation info, etc.), even
a passive MITM could steal the info anyway.  Also, if the major
real-world MITM we're talking about is someone intercepting a
non-secured wireless network, the attacker already has physical
proximity, so he a) knows approximate geolocation info and b) could
possibly take a picture by more conventional means.

If I understand correctly, I am not at all sure that the increased
user annoyance is worth the increased protection of user privacy.
These permissions can always get abused anyway by someone hacking the
site the user has given permission to, and in my experience that's
considerably more common than a MITM attack, so users can't really
depend on the permissions not being abused anyway.  The incremental
reduction of attack surface doesn't seem to gain us a lot.

I definitely think that there is no basis at all for disabling pop-up
permissions or other things that only affect user convenience.  Just
because there's an MITM doesn't mean it warrants being treated as a
security issue -- it's a tradeoff of user convenience vs. user
convenience, and it's obvious that user convenience is better served
by allowing the pop-up permission to be remembered.  Privacy vs.
convenience is a less obvious tradeoff to make.

(By the way, I'm very alarmed by your implication that a site with
persisted camera permissions can take pictures whenever it wants.  All
it would take is one reasonably large site getting hacked, and tens of
thousands of people could receive Give me $100 or I'll post pictures
of you in your underwear all over the Internet threats.  I'm much
more worried about server-side hacking or abuse than MITM.  I've had
sites I subscribe to hacked multiple times, and one time a site I ran.
I don't think I've even been subject to a real MITM attack.)
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to deprecate: persistent permissions over HTTP

[Aryeh Gregor]

On Fri, Mar 6, 2015 at 7:27 PM, Anne van Kesteren ann...@annevk.nl wrote:
 A large number of permissions we currently allow users to store
 persistently for a given origin. I suggest we stop offering that
 functionality when there's no lock in the address bar. This will make
 it harder for a network attacker to abuse these permissions. This
 would affect UX for:

 * Geolocation
 * Notification
 * Fullscreen
 * Pointer Lock
 * Popups

What attack is this designed to mitigate?  If the user allows an
unsecured site to use (for instance) geolocation, whether persisted or
not, an MITM will be able to get the geolocation info as long as
they're intercepting the traffic, right?  And if they have some way to
persist their scripts via injecting modified resources with long cache
timeouts or such, they can still get the info as long as the user
keeps clicking yes.  And the user will definitely keep clicking yes,
because a) they clicked it the first time, and b) you have conditioned
them to click yes a million times on the same site.  So how does not
persisting this info help at all?  Probably I'm missing something
obvious.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Thread-Safe DOM // was Re: do not deprecate synchronous XMLHttpRequest

[Aryeh Gregor]

On Thu, Feb 12, 2015 at 4:45 AM, Marc Fawzi marc.fa...@gmail.com wrote:
 how long can this be sustained? forever? what is the point in time where the
 business of retaining backward compatibility becomes a huge nightmare?

It already is, but there's no way out.  This is true everywhere in
computing.  Look closely at almost any protocol, API, language, etc.
that dates back 20 years or more and has evolved a lot since then, and
you'll see tons of cruft that just causes headaches but can't be
eliminated.  Like the fact that Internet traffic is largely in
1500-byte packets because that's the maximum size you could have on
ancient shared cables without ambiguity in the case of collision.  Or
that e-mail is mostly sent in plaintext, with no authentication of
authorship, because that's what made sense in the 80s (or whatever).
Or how almost all web traffic winds up going over TCP, which performs
horribly on all kinds of modern usage patterns.  For that matter, I'm
typing this with a keyboard layout that was designed well over a
century ago to meet the needs of mechanical typewriters, but it became
standard, so now everyone uses it due to inertia.

This is all horrible, but that's life.

Re: [Selection] Should selection.getRangeAt return a clone or a reference?

[Aryeh Gregor]

On Tue, Jan 27, 2015 at 4:49 PM, Koji Ishii kojii...@gmail.com wrote:
 3 proposals so far:

 Proposal A: Leave it undefined. If it's not causing interop issues, we
 can leave it.
 Proposal B: Clone.
 Proposal C: Live.

I can live with any, but prefer B.

Re: HTTP/2 and User-Agent strings?

[Aryeh Gregor]

On Tue, Jan 27, 2015 at 11:31 PM, Chris Peterson cpeter...@mozilla.com wrote:
 Are there recent studies of which features servers do detect and why? I
 could see arguments for sharing information about mobile devices, touch
 support, and OS.

Long ago I used to do development for MediaWiki.  We had UA string
checks to work around certain UA-specific bugs where no other easy
workaround was possible.  I remember one particular bug in Firefox
(which was only fixed years later) that I spent considerable effort
trying to figure out how to work around without a UA string check, and
eventually gave up.  It was something like: if you append #foo to the
URL while the page is loading in order to jump to span id=foo, but
that element has not yet loaded when you set the URL, every other
browser would jump to the element when it was added to the DOM, but
Gecko would not.  So I checked for Gecko and set the hash again
onload.  I didn't want to do this for non-Gecko browsers, because it
would degrade the user experience (delaying the jump until the whole
page loaded).  And I couldn't figure out any reasonable way to detect
it without UA sniffing.

Sometimes browser behavior is just not reasonably detectable other
than by UA sniffing.  Yes, authors should use other methods if
practical, and when I was a web dev I always would, but sometimes it's
just not practical.  Sometimes it's not even possible.  This
particular example did UA sniffing from JavaScript, not HTTP headers,
but it illustrates why authors do need access to this information, and
always will.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: [Selection] Support of Multiple Selection (was: Should selection.getRangeAt return a clone or a reference?)

[Aryeh Gregor]

On Tue, Jan 27, 2015 at 4:31 PM, Koji Ishii kojii...@gmail.com wrote:
 It's true that you could use multi-range selections to select in
 visual order. But there are bunch of operations defined against
 selections. What does it copy? What will happen when you type a to
 replace the selected text? Spell check? Bi-di algorithm? Almost every
 text algorithm is built on top of the model, which is the DOM today,
 we can't just replace it.

In all of these cases, typically, the most correct thing you can do is
do the operation on each range separately in sequence, probably in DOM
order for lack of a better option.  Copy should concatenate the
selected ranges into the clipboard.  Replacement probably would delete
all the ranges and replace the first one.  I don't see what spellcheck
or bidi have to do with selections at all.  All this is certainly not
simple to work out, and in some cases there will be no good answer for
what to do, but it's something you have to do if you want to deal with
non-contiguous selections.

 I think visual order selections, if ever happens, should have a
 different architecture, and it should not be handled together with
 multi-range selections.

What do you mean by visual-order selections, and can you give a
specific example of something that should behave differently for
visual-order and multi-range selections?

Re: [Selection] Support of Multiple Selection (was: Should selection.getRangeAt return a clone or a reference?)

[Aryeh Gregor]

On Sat, Jan 24, 2015 at 9:18 PM, Tab Atkins Jr. jackalm...@gmail.com wrote:
 Though I believe browsers will soon have much more pressure to support
 multiple ranges as a matter of course, as increased design with
 Flexbox and Grid will mean that highlighting from one point to
 another, in the world of a range is defined by two DOM endpoints and
 contains everything between them in DOM order, can mean highlighting
 random additional parts of the page that are completely unexpected.
 Switching to a model of visual highlighting for selections will
 require multi-range support.

 In other words, it'll switch from being a rare thing to much more common.

Most sites will probably not use flexbox or grid for a long time to
come, and on sites that do non-contiguous selections will probably be
rare, so I wouldn't rely on this as a mitigating factor.  I once went
through some common selection use-cases with the new selection API
that I suggested before (returning a list of selected nodes or such),
and for at least some common cases (like wrap the selection in a
span) it handled non-contiguous selections automatically, and was
easier to use as well.  For typical selection use-cases, the author
wants to deal with the selected nodes anyway, not the endpoints.

Re: [Selection] Should selection.getRangeAt return a clone or a reference?

[Aryeh Gregor]

On Sun, Jan 25, 2015 at 1:31 AM, Mats Palmgren m...@mozilla.com wrote:
 Gecko knows if a Range is part of a Selection or not.

Authors don't, I don't think.  Of course, we could expose this info to
authors if we wanted, so that's not a big problem.

 True, I'm just saying that I don't see any practical problems in
 implementing live ranges to manipulate the Selection if we want to.

I don't think there are any implementation problems, I just think it's
an API that's confusing to authors relative to the alternative
(returning copies).  And it's probably easier for the UAs that return
references to switch to returning copies than the reverse, so it
increases the chance of convergence in the near term.  Also, if
mutating the range throws, it will break author code; but if it fails
silently, it creates a what on earth is going wrong?! head-banging
scenario for authors.  And anything authors can do with a reference,
they can do with a copy just as well, by mutating the copy,
.removeRange(), .addRange().  So I think returning a copy makes much
more sense.