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

2017-09-12 Thread Boris Zbarsky

On 9/12/17 5:04 PM, Emilio Cobos Álvarez wrote:

I've noticed that this may be used pretty easily for UA detection.


Right, that and use in Gecko-only codepaths are the main concerns

I considered adding a usecounter, but as you noted it would be affected 
by window enumeration.


We could make the property non-enumerable and _then_ add a usecounter. 
It would take a bit of machinery (e.g. support for non-enumerable webidl 
attributes), but not that hard.


We could also delay the removal to after 57 to mitigate 57 risk

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


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

2017-09-12 Thread Kohei Yoshino

A similar story: `window.controllers` was removed with Firefox 29 but added 
back to Firefox 30 because it had been widely used for UA detection. 
`window.content` might cause the same compatibility issue, but anyway, it's 
difficult to guess the impact from GitHub search results...

https://www.fxsitecompat.com/en-CA/docs/2014/window-content-controllers-pkcs11-and-loadstatus-have-been-removed/

-Kohei


On 2017-09-12 5:04 PM, Emilio Cobos Álvarez wrote:

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

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

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

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

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

  -- Emilio

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

On 09/12/2017 09:32 PM, Boris Zbarsky wrote:

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

window.content is a Gecko-specific thing that basically acts like
window.top in untrusted code.  In chrome it returns the currently
selected tab, effectively.

I would like to unship window.content for 57; no one else implements it.

-Boris

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


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

2017-09-12 Thread Emilio Cobos Álvarez
Just for the record, since I got curious and I saw no mention in the
intent email:

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

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

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

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

 -- Emilio

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

On 09/12/2017 09:32 PM, Boris Zbarsky wrote:
> Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=864845
> 
> window.content is a Gecko-specific thing that basically acts like
> window.top in untrusted code.  In chrome it returns the currently
> selected tab, effectively.
> 
> I would like to unship window.content for 57; no one else implements it.
> 
> -Boris
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


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

2017-09-12 Thread Boris Zbarsky

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

window.content is a Gecko-specific thing that basically acts like 
window.top in untrusted code.  In chrome it returns the currently 
selected tab, effectively.


I would like to unship window.content for 57; no one else implements it.

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


Re: Intermittent oranges and when to disable the related test case - a simplified policy

2017-09-12 Thread Steve Fink

On 9/12/17 7:02 AM, James Graham wrote:

On 12/09/17 14:55, Andrew Halberstadt wrote:
On Mon, Sep 11, 2017 at 10:33 PM Robert O'Callahan 


wrote:


On Tue, Sep 12, 2017 at 11:38 AM, Andrew Halberstadt <
ahalberst...@mozilla.com> wrote:


I don't think so, that data already exists and is query-able from
ActiveData:
https://activedata.allizom.org/tools/query.html#query_id=8pDOpeni



That query tells you about disabled tests, but doesn't know about 
*why* a

test was disabled. E.g. you can't distinguish tests disabled because
they're not expected to work on some (or all) platforms from tests that
were disabled for intermittent failures that should, in principle, 
be fixed.


Rob



True, though I don't know that gps' proposal would solve that either.

But this is a good idea, and is easy to solve from a technical 
standpoint.

We'd just need to agree on some standard manifest keys:


I'm pretty sure that the problem isn't technical, but actually getting 
people to do that consistently (plus retrofitting the data onto 
thousands of currently disabled tests). You would at least have to add 
a lint and a free pass for all the existing tests. 


If you don't need to get it perfect, you can always do an hg blame and 
look at the commit message that disabled a test, then test it for 
whatever makes sense -- mentioning "intermittent", the author being a 
sheriff, or something similar. Or at least gathering them all together 
so you can skim through them and mark which ones sound like they were 
disabled for being intermittent.


I'm not just handwaving here, or at least not completely -- I have 
something like this set up for semi-automatically generating 
Spidermonkey release notes on changed API functions, where it looks at 
the revision where each function was added or removed and adds the bug 
number to the appropriate list, with some attempt made to avoid 
whitespace changes and reordering within the header files. (It doesn't 
work very well, mind.)


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


Re: indexedDB.open failing silently?

2017-09-12 Thread Boris Zbarsky

On 9/12/17 5:57 AM, Geoff wrote:

What they got instead was: undefined.


I actually can't think of a way this could happen...

This surely must mean that indexedDB.open is failing but not throwing an 
exception.


Failing in what sense?

There _is_ a concept of "uncatchable exception" in Gecko, but it would 
still get reported to the console by the console evaluation machinery. 
You can try this out:


  try {
while(1);
'finished';
  } catch(ex) {
console.error(ex);
  }

and then click "Stop it" when the infobar about the looping script 
appears.  Note the error report logged to the console.


So if open() does not terminate execution, you should get "finished". 
If it terminates via an exception, you should get into the catch block 
and console.error.  If it terminates via "uncatchable exception", the 
console machinery should report that.  In no case should you get bare 
`undefined` as a result of evaluating your pasted script...


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


Re: indexedDB.open failing silently?

2017-09-12 Thread Jan Varga
I just tried to simulate the problem with newer/older firefox, but with 
given code snippet I get an error event instead of "undefined"


It's probably something else ...


On 12/09/17 15:56, Ben Kelly wrote:

Can you have the user try in a fresh profile?

I think this behavior might occur if they used a profile in a newer version
of firefox (like nightly 57) and then try to take it back to an older
version (like release 55).  Database schemas can be updated in various
storage APIs on disk which prevent older versions of firefox from opening
the any storage for the origin.

On Tue, Sep 12, 2017 at 5:57 AM, Geoff  wrote:


I'm trying to help some users of my extension with a problem.
Unfortunately I can only offer suggestions of things to try and then wait
for a reply.

I asked one to put this code in the web console and tell me what happened:

try {
 var request = indexedDB.open('thisIsATest', 1);
 request.onsuccess = console.log;
 request.onerror = console.error;
 request.onblocked = console.error;
 request.onupgradeneeded = console.log;
 'finished';
} catch(ex) {
 console.error(ex);
}

What you'd expect to see on the console is either "finished" and then one
or more messages from the event handlers, or an error from the catch block.

What they got instead was: undefined.

This surely must mean that indexedDB.open is failing but not throwing an
exception. I've tried disabling indexedDB, or making the appropriate
directory read-only, but in both cases an error is reported (even if it is,
helpfully, "UnknownError").

So I'm stuck. What could be causing this, and what can I do about it?

(If you want to chip in on the issue directly:
https://github.com/darktrojan/newtabtools/issues/307)

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


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


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


Re: Intermittent oranges and when to disable the related test case - a simplified policy

2017-09-12 Thread James Graham

On 12/09/17 14:55, Andrew Halberstadt wrote:

On Mon, Sep 11, 2017 at 10:33 PM Robert O'Callahan 
wrote:


On Tue, Sep 12, 2017 at 11:38 AM, Andrew Halberstadt <
ahalberst...@mozilla.com> wrote:


I don't think so, that data already exists and is query-able from
ActiveData:
https://activedata.allizom.org/tools/query.html#query_id=8pDOpeni



That query tells you about disabled tests, but doesn't know about *why* a
test was disabled. E.g. you can't distinguish tests disabled because
they're not expected to work on some (or all) platforms from tests that
were disabled for intermittent failures that should, in principle, be fixed.

Rob



True, though I don't know that gps' proposal would solve that either.

But this is a good idea, and is easy to solve from a technical standpoint.
We'd just need to agree on some standard manifest keys:


I'm pretty sure that the problem isn't technical, but actually getting 
people to do that consistently (plus retrofitting the data onto 
thousands of currently disabled tests). You would at least have to add a 
lint and a free pass for all the existing tests.

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


Re: indexedDB.open failing silently?

2017-09-12 Thread Ben Kelly
Can you have the user try in a fresh profile?

I think this behavior might occur if they used a profile in a newer version
of firefox (like nightly 57) and then try to take it back to an older
version (like release 55).  Database schemas can be updated in various
storage APIs on disk which prevent older versions of firefox from opening
the any storage for the origin.

On Tue, Sep 12, 2017 at 5:57 AM, Geoff  wrote:

> I'm trying to help some users of my extension with a problem.
> Unfortunately I can only offer suggestions of things to try and then wait
> for a reply.
>
> I asked one to put this code in the web console and tell me what happened:
>
> try {
> var request = indexedDB.open('thisIsATest', 1);
> request.onsuccess = console.log;
> request.onerror = console.error;
> request.onblocked = console.error;
> request.onupgradeneeded = console.log;
> 'finished';
> } catch(ex) {
> console.error(ex);
> }
>
> What you'd expect to see on the console is either "finished" and then one
> or more messages from the event handlers, or an error from the catch block.
>
> What they got instead was: undefined.
>
> This surely must mean that indexedDB.open is failing but not throwing an
> exception. I've tried disabling indexedDB, or making the appropriate
> directory read-only, but in both cases an error is reported (even if it is,
> helpfully, "UnknownError").
>
> So I'm stuck. What could be causing this, and what can I do about it?
>
> (If you want to chip in on the issue directly:
> https://github.com/darktrojan/newtabtools/issues/307)
>
> GL
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Intermittent oranges and when to disable the related test case - a simplified policy

2017-09-12 Thread Andrew Halberstadt
On Mon, Sep 11, 2017 at 10:33 PM Robert O'Callahan 
wrote:

> On Tue, Sep 12, 2017 at 11:38 AM, Andrew Halberstadt <
> ahalberst...@mozilla.com> wrote:
>
>> I don't think so, that data already exists and is query-able from
>> ActiveData:
>> https://activedata.allizom.org/tools/query.html#query_id=8pDOpeni
>
>
> That query tells you about disabled tests, but doesn't know about *why* a
> test was disabled. E.g. you can't distinguish tests disabled because
> they're not expected to work on some (or all) platforms from tests that
> were disabled for intermittent failures that should, in principle, be fixed.
>
> Rob
>

True, though I don't know that gps' proposal would solve that either.

But this is a good idea, and is easy to solve from a technical standpoint.
We'd just need to agree on some standard manifest keys:

[test_foo.html]
skip-if = 
reason = {'intermittent', 'fail', ... }
bugs = 1234567, 1234576

When we log the skip, we make sure this metadata makes it into the
structured log. Then tools like ActiveData or a manifest parsing mach
command can query it. Reftest and wpt complicate matters a bit, but we
could figure out something similar for them.

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


indexedDB.open failing silently?

2017-09-12 Thread Geoff
I'm trying to help some users of my extension with a problem. 
Unfortunately I can only offer suggestions of things to try and then 
wait for a reply.


I asked one to put this code in the web console and tell me what happened:

try {
var request = indexedDB.open('thisIsATest', 1);
request.onsuccess = console.log;
request.onerror = console.error;
request.onblocked = console.error;
request.onupgradeneeded = console.log;
'finished';
} catch(ex) {
console.error(ex);
}

What you'd expect to see on the console is either "finished" and then 
one or more messages from the event handlers, or an error from the catch 
block.


What they got instead was: undefined.

This surely must mean that indexedDB.open is failing but not throwing an 
exception. I've tried disabling indexedDB, or making the appropriate 
directory read-only, but in both cases an error is reported (even if it 
is, helpfully, "UnknownError").


So I'm stuck. What could be causing this, and what can I do about it?

(If you want to chip in on the issue directly: 
https://github.com/darktrojan/newtabtools/issues/307)


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


Re: Re-visiting the DOM tree depth limit in layout

2017-09-12 Thread Henri Sivonen
On Tue, Sep 12, 2017 at 12:35 PM, Henri Sivonen  wrote:
> I'm rather unhappy about the prospect of having to examine another
> browser's HTML parser beyond reading the spec in order to achieve
> interop. :-(

Fortunately, not too complicated:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/html/parser/HTMLConstructionSite.cpp?sq&l=306

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


Re: Re-visiting the DOM tree depth limit in layout

2017-09-12 Thread Henri Sivonen
On Mon, Sep 11, 2017 at 5:37 PM, Geoffrey Sneddon  wrote:
> Chrome limits the DOM at 513 deep (where the root element is one
> deep), Safari likewise but doesn't paint anything. Edge appears to
> have no limit, so that TC simply hangs in an infinite loop (presumably
> it will eventually go OOM and crash, though I wonder if its
> performance isn't linear?).

Indeed, Chrome has a depth limit of 513 elements including the root
(with the depth extending to 514 when counting the text node leaves).

However, Blink does something more complex than Gecko in order to
enforce the limit. Consider the appearance of
https://hsivonen.com/test/moz/deeptree/font/4000.html in Firefox and
Chrome. The visual formatting of the page is as if Chrome didn't have
a depth limit.

I'm rather unhappy about the prospect of having to examine another
browser's HTML parser beyond reading the spec in order to achieve
interop. :-(

On Mon, Sep 11, 2017 at 5:24 PM, Boris Zbarsky  wrote:
> On 9/11/17 10:08 AM, Henri Sivonen wrote:
>>
>> What kind of style would have the maximum stack frame size? Is
>> display: table-cell enough to trigger the worst case?
>
>
> I suspect that display:table-cell is the worst case, but at only about 70%
> confidence.
>
> That said, I can't think of a worse case offhand.

Indeed, when the divs are styled as display: table-cell, the stack
overflows sooner than in the default styling case. So much so that our
current limit of 200 isn't enough to keep 32-bit Windows builds from
crashing with a stack overflow.

With the tests at https://hsivonen.com/test/moz/deeptree/ today, I see
the following:
Linux 64 (Ubuntu 16.04):
Plain div crashes between 3070 and 3080.
Div as table-cell crashes between 1080 and 1090.
The font crash threshold is somewhere above 4000.

Windows 64 (Windows 10):
Plain div crashes between 740 and 750.
Div as table-cell crashes between 240 and 250.
font crashes between 1910 and 1920

WoW (Windows 10):
Plain div crashes between 510 and 520.
Div as table-cell crashes somewhere below 200.
font crashes between 1480 and 1490

On Mon, Sep 11, 2017 at 6:39 PM, Jan de Mooij  wrote:
> On Mon, Sep 11, 2017 at 5:27 PM,  wrote:
>
>> On Monday, September 11, 2017 at 1:41:43 PM UTC+2, Henri Sivonen wrote:
>> > Can we ask Windows to give us more stack space?
>> > --
>> Don't know if it's possible to set it dynamically at runtime, but Visual
>> Studio supports setting it at compile time (in linker options).
>>
>
> Yes, we already set the stack size to 2 MB on Win64 [0].

Is there a reason not to increase the stack size on 32-bit Windows?

Is there a reason not to increase the stack size on 64-bit Windows to
the Linux/Mac ballpark?

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