735 bytes, text/html
19.95 KB, text/html
1.06 KB, patch
|Details | Diff | Splinter Review|
Created attachment 664226 [details] testcase (crashes Firefox when loaded) bp-0cada683-3e71-482d-8d98-0c2d92120924 I'm guessing this is a regression from bug 778810, which added the function mozilla::ScrollbarActivity::ActivityOccurred. Why does scrolling set DOM attributes?
Why do we end up dispatching mutation event. Scrollbars should be native anonymous.
Created attachment 664662 [details] [diff] [review] patch Shouldn't rely on the element type, but frame type. In the test we end up getting parent frame which is ScrollFrame.
As far as I see, this is a really old bug.
I loaded the testcase on Windows: * 15.0.1: no crash * 16.0b4: no crash * 17.0a2: bp-101eb84b-d2b6-4f41-9dbf-37a5c2120926 * 18.0a1: bp-5894bccb-d4ac-448a-b32b-4c32d2120926
Sure, using *that* testcase. But I don't see why the problem wouldn't happen with some other testcase too.
Comment on attachment 664662 [details] [diff] [review] patch r=me
Comment on attachment 664662 [details] [diff] [review] patch [Security approval request comment] How easily can the security issue be deduced from the patch? I'd say quite easily. Just try to make svg:use to use some unusual frame type Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? NA Which older supported branches are affected by this flaw? All Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? The same patch should work in older branches too. (if the patch doesn't apply cleanly, the changes should be minor syntactic changes. The code is several years old.) How likely is this patch to cause regressions; how much testing does it need? Should be reasonable safe. In normal cases svg:use should behave like svg:use.
Comment on attachment 664662 [details] [diff] [review] patch sec-approval+ assuming you can get branch landing approval from drivers
(In reply to Olli Pettay [:smaug] from comment #8) > Which older supported branches are affected by this flaw? > All Even though we don't crash in 15/16? Why are those branches marked as unaffected? We're leaning towards not taking this for FF16 to limit risk, since this was internally reported.
So I can certainly get access to supposed-to-be-native-anonymous element in FF16. So far no crash, but I don't see why.
If we don't take this to FF16, should we land the patch closer to FF17 release?
Comment on attachment 664662 [details] [diff] [review] patch Longstanding issue, internally reported, and FF16 doesn't crash. I don't think this qualifies for our final beta. Setting the sec-approval flag back to ? to answer the question of when to land on Central/Aurora/(next Beta).
We'll want a test for this that we can check in once we've release Fx17.
Comment on attachment 664662 [details] [diff] [review] patch sec-approval+, re-requesting branch approvals.
https://hg.mozilla.org/releases/mozilla-beta/rev/e77259feea47 https://hg.mozilla.org/releases/mozilla-aurora/rev/dcc60baeca75 https://hg.mozilla.org/releases/mozilla-esr10/rev/92ef4d0a9c88
mass remove verifyme requests greater than 4 months old