Currently, our internal event handler for implementing default action calls preventDefault() for preventing other internal event handlers not handling the event anymore. I think that this is okay. However, this causes defaultPrevented attribute becomes true for web apps (content) too. I'm thinking that we can quick hack for this. 1. Before dispatching an event in system group, store the defaultPrevented value. If it becomes true after dispatched, we should set new flag. http://mxr.mozilla.org/mozilla-central/source/content/events/src/nsEventDispatcher.cpp#342 2. Before calling nsIFrame::HandleEvent(), store the defaultPrevented value. If it becomes true after the call, we should set new same flag. http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp#477 3. If the new flag and defaultPrevented flag is true while the event phase is NONE, nsIDOMWindow::DefaultPrevented() should return false only when the caller is content. Even with this hack, if some default actions are implemented in normal event group, this hack doesn't work. However, such handlers should be moved to system group because such handlers may be suppressed by stopPropagation() of web apps.

This patch adds mDefaultPreventedByDefaultActionHandler to EventFlagsBase. This stores if mDefaultPrevented is set in system group (see nsDOMEvent::PreventDefault()). In nsDOMEvent::DefaultPrevented(), the new member is used for deciding the result. See the comments in the method for the detail. The risky point of this bug is, nsIFrame::HandleEvent() is called in system group instead of normal group. I have no idea about regressions which are caused by this change, though.

Comment on attachment 824060 [details] [diff] [review] Patch >+ // While this event is being dispatched, returns true if PreventDefault() has >+ // been called by anybody since any other event handler shouldn't handle such >+ // event anymore. >+ // XXX This may return unexpected value if two or more events are nested and >+ // preceding event is stored and the nested event handler referes it. refers >+ // For example, when a keypress event causes modifying the text, an input >+ // event may be dispatched from keypress event handler of editor. >+ // I.e., when an input event handler runs, the keypress event is >+ // still being dispatched. However, preventDefault attribute of the >+ // keypress event (if it was stored a variable at preceding keypress event >+ // handler) returns true even though the content has never called >+ // preventDefault() of the keypress event. >+ // However, this is really rare case, we don't need to mind it. Nested events are not a rare case and we need to figure out some way to deal with this situation.

(In reply to Olli Pettay [:smaug] from comment #2) > >+ // For example, when a keypress event causes modifying the text, an input > >+ // event may be dispatched from keypress event handler of editor. > >+ // I.e., when an input event handler runs, the keypress event is > >+ // still being dispatched. However, preventDefault attribute of the > >+ // keypress event (if it was stored a variable at preceding keypress event > >+ // handler) returns true even though the content has never called > >+ // preventDefault() of the keypress event. > >+ // However, this is really rare case, we don't need to mind it. > Nested events are not a rare case and we need to figure out some way to deal > with this situation. Nested event is not a rare case, but I meant referring other events from an event handler is a rare case. (I.e., referring keypress event's defaultPrevented from input event handler) In this example, we can fix this issue by making editor call preventDefault() *after* dispatch an input event. So, I think that we need to check all event handlers one by one. I don't think that it's scope of this bug. There is another possible idea, but I'm not sure this works fine in all cases. If nsDOMEvent::DefaultPrevented() is called only from JS, we can check if the caller is chrome. If the caller is chrome, it should return mDefaultPrevented. Otherwise, check the new member. And then, for compatibility, GetDefaultPrevented() and GetPreventDefault() always return mDefaultPrevented. So, the point which I'm not sure is, whether DefaultPrevented() is called only from script or not and GetDefaultPrevented() is called only from C++ or not.

We can have JS-only version of DefaultPrevented() if needed. Just map the webidl's defaultPrevented to some FooBarJS() method using Bindings.conf

Thank you for the really helpful information. I'm writing this patch. The test result is here: https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=616c4e3b41aa Then, I hit a problem with following tests: http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/text/tests/test_texteditor_keyevent_handling.html?force=1 http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/html/tests/test_htmleditor_keyevent_handling.html?force=1 They are mochitest-plain tests. Therefore, even in the system event listeners which are registered with SpecialPowers, the defaultPrevented returns false for content script. If we can return default action result in such case, we can keep the compatibility perfectly. I think that if we can do it, it's the best solution. However, for it, nsDOMEvent::DefaultPreventedJS() needs to know whether the event is the latest dispatched event or not. I.e., nsEventListenerManager ot somebody needs to store the pointer to DOM event at dispatching it. But I'm not sure what way is thread-safe. If nsEventListenerManager were created for each thread, it'd fix this issue easily. But looks like it's not so. Do you have any idea for this? An alternative hack is, we make the tests use getPreventDefault() instead of defaultPrevented. I think that modern apps should use only defaultPrevented. Therefore, I think that not changing getPreventDefault() behavior is better choice. Then, this way can fix the issue. # I'm still not sure a cause of the orange of 5 on Mac, though.

Or, the tests should be switched back to mochitest-chrome again because they test chrome level behavior.

Perhaps, we should make the editor tests chrome tests.

Hmm, by making test_assign_event_data.html a content test, it causes new random orange on Mac :-( See the screenshot, then, you can understand the reason. The preceding test test_handlerApps.xhtml launches iCal and kill it. However, looks like it fails. http://mxr.mozilla.org/mozilla-central/source/uriloader/exthandler/tests/mochitest/test_handlerApps.xhtml http://mxr.mozilla.org/mozilla-central/source/uriloader/exthandler/tests/mochitest/handlerApps.js I think that I should file a bug for this issue and disables test_assign_event_data.html only on Mac...

Comment on attachment 825282 [details] [diff] [review] part.1 Event.defaultPrevented should be false even if preventDefault() has been called by default action handler This approach is, if the caller of defaultPrevented is content, it hides the fact that the event is prevented other actions by default action handler. Otherwise, i.e., chrome can know if the preventDefault() has already been called.

Comment on attachment 825349 [details] [diff] [review] part.2 Fix test_assign_event_data.html for new defaultPrevented behavior and make it a chrome test This test should be mochitest-plain because it tests the behavior for web applications. Additionally, this test tests calling preventDefault() from content case too.

Comment on attachment 825284 [details] [diff] [review] part.3 Make test_*editor_keyevent_handling.html chrome tests since they tests behavior of system group event handler These events test if preventDefault() is called by default action handler in editor. Therefore, these events should be mochitest-chrome since the default action handler behavior is hidden from defaultPrevented attribute.

Comment on attachment 825285 [details] [diff] [review] part.4 Fix new orange of test_bug448987.html on Mac since it doesn't set tab navigation setting I'm not sure why this test becomes orange on Mac by my patches. The reason is, the test doesn't set accessibility.tabfocus for making image map tabbable on Mac. This patch just sets the pref and fix a bug when it fails to set focus by tab navigation. (Currently, if it fails, error message is printed repeatedly since the interval timer isn't canceled)

Comment on attachment 825282 [details] [diff] [review] part.1 Event.defaultPrevented should be false even if preventDefault() has been called by default action handler I don't know what "default event action handler" is. Why not just mDefaultPreventedByDefaultHandler (that would mean anything in system event group or PostHandleEvent) But I need to think this all a bit. I don't like relying on nsContentUtils::ThreadsafeIsCallerChrome() calls.

Comment on attachment 825284 [details] [diff] [review] part.3 Make test_*editor_keyevent_handling.html chrome tests since they tests behavior of system group event handler Though this is a bit worrisome, since whatever the test is doing isn't apparently possible after the change... but yeah, that is what the bug is about, to change the web facing API.

Comment on attachment 825285 [details] [diff] [review] part.4 Fix new orange of test_bug448987.html on Mac since it doesn't set tab navigation setting This could land separately before any other patches, right?

Comment on attachment 825349 [details] [diff] [review] part.2 Fix test_assign_event_data.html for new defaultPrevented behavior and make it a chrome test I don't understand the changes to runNextTest(), and the timeout is scary. refresh driver may not run within 100ms if we happen to run the test on a debug build and GC/CC runs after the setTimeout call. If you need refresh driver checks, use requestAnimationFrame

Comment on attachment 825282 [details] [diff] [review] part.1 Event.defaultPrevented should be false even if preventDefault() has been called by default action handler So could we actually use implicitJSContext in the Bindings.conf and have a JS only version of PreventDefault(). Then use the passed cx for the chrome check, and C++ callers would always mean "default handler". Also, doesn't DefaultPreventedJS() return wrong value if content calls preventDefault() and then default handler / chrome calls it too.

(In reply to Olli Pettay [:smaug] from comment #21) > So could we actually use implicitJSContext in the Bindings.conf and have > a JS only version of PreventDefault(). Then use the passed cx for the > chrome check, and C++ callers would always mean "default handler". How can I check if the context is in content or chrome with JSContext? JSContext::runningWithTrustedPrincipals() sounds a method which I want. However, it's used only in under js. So, it looks a private method in the js module. What's the way you expected?

xpc::AccessCheck::isChrome(js::GetContextCompartment(cx))

(In reply to Olli Pettay [:smaug] from comment #23) > xpc::AccessCheck::isChrome(js::GetContextCompartment(cx)) Hmm, that causes crash in tests of workers only on debug build: http://mxr.mozilla.org/mozilla-central/source/caps/include/nsJSPrincipals.h#25 > 22:01:44 INFO - 1912 INFO TEST-PASS | /tests/dom/workers/test/test_dataURLWorker.html | Got correct message > 22:01:44 INFO - 1913 INFO TEST-INFO | MEMORY STAT vsize after test: 3758231552 > 22:01:44 INFO - 1914 INFO TEST-INFO | MEMORY STAT residentFast after test: 296636416 > 22:01:44 INFO - 1915 INFO TEST-END | /tests/dom/workers/test/test_dataURLWorker.html | finished in 137ms > 22:01:44 INFO - DefaultPrevented(JSContext*) > 22:01:44 INFO - ++DOMWINDOW == 27 (0x129296428) [pid = 894] [serial = 611] [outer = 0x12670e118] > 22:01:44 INFO - 1916 INFO TEST-START | /tests/dom/workers/test/test_errorPropagation.html > 22:01:44 INFO - DefaultPrevented(JSContext*) > 22:01:44 INFO - DefaultPrevented(JSContext*) > 22:01:44 INFO - ++DOMWINDOW == 28 (0x1479da9b8) [pid = 894] [serial = 612] [outer = 0x12670e118] > 22:01:44 INFO - ++DOCSHELL 0x11ce591e0 == 9 [pid = 894] [id = 163] > 22:01:44 INFO - ++DOMWINDOW == 29 (0x1283bf788) [pid = 894] [serial = 613] [outer = 0x0] > 22:01:44 INFO - [894] WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && subjPrincipal) failed: file ../../../docshell/base/nsDocShell.cpp, line 8591 > 22:01:44 INFO - ++DOMWINDOW == 30 (0x11dbe5a58) [pid = 894] [serial = 614] [outer = 0x1283bf788] > 22:01:44 INFO - 1917 INFO TEST-PASS | /tests/dom/workers/test/test_errorPropagation.html | Correct message event.message > 22:01:44 INFO - 1918 INFO TEST-PASS | /tests/dom/workers/test/test_errorPropagation.html | Correct message event.filename > 22:01:44 INFO - 1919 INFO TEST-PASS | /tests/dom/workers/test/test_errorPropagation.html | Correct message event.lineno > 22:01:44 INFO - PreventDefault(JSContext*) > 22:01:44 INFO - Assertion failure: self->debugToken == DEBUG_TOKEN, at ../../../dist/include/nsJSPrincipals.h:25 > 22:01:46 WARNING - TEST-UNEXPECTED-FAIL | /tests/dom/workers/test/test_errorPropagation.html | application terminated with exit code 256 > 22:01:46 INFO - INFO | runtests.py | Application ran for: 0:01:27.596645 > 22:01:46 INFO - INFO | zombiecheck | Reading PID log: /var/folders/99/99J8qVNwGhyCKSm1qW-gHE+++-k/-Tmp-/tmpVnBh5Kpidlog > 22:02:05 WARNING - PROCESS-CRASH | /tests/dom/workers/test/test_errorPropagation.html | application crashed [@ xpc::AccessCheck::isChrome(JSCompartment*)] > 22:02:05 INFO - Crash dump filename: /var/folders/99/99J8qVNwGhyCKSm1qW-gHE+++-k/-Tmp-/tmpbx7wgR/minidumps/EA8C500F-795B-40E7-A1B9-23973D69F048.dmp > 22:02:05 INFO - Operating system: Mac OS X > 22:02:05 INFO - 10.6.8 10K549 > 22:02:05 INFO - CPU: amd64 > 22:02:05 INFO - family 6 model 23 stepping 10 > 22:02:05 INFO - 2 CPUs > 22:02:05 INFO - Crash reason: EXC_BAD_ACCESS / KERN_INVALID_ADDRESS > 22:02:05 INFO - Crash address: 0x0 > 22:02:05 INFO - Thread 49 (crashed) > 22:02:05 INFO - 0 XUL!xpc::AccessCheck::isChrome(JSCompartment*) [nsJSPrincipals.h:6735250b8900 : 25 + 0x0] > 22:02:05 INFO - rbx = 0x00007fff7000a2f8 r12 = 0x000000011936af00 > 22:02:05 INFO - r13 = 0x0000000149bff53a r14 = 0x0000000127cf8600 > 22:02:05 INFO - r15 = 0x000000010224f400 rip = 0x000000010282287a > 22:02:05 INFO - rsp = 0x0000000149bfec20 rbp = 0x0000000149bfec40 > 22:02:05 INFO - Found by: given as instruction pointer in context > 22:02:05 INFO - 1 XUL!nsDOMEvent::PreventDefault(JSContext*) [nsDOMEvent.cpp:6735250b8900 : 455 + 0x4] > 22:02:05 INFO - rbx = 0x000000011936afe0 r12 = 0x000000011936af00 > 22:02:05 INFO - r13 = 0x0000000149bff53a r14 = 0x0000000127cf8680 > 22:02:05 INFO - r15 = 0x000000010224f400 rip = 0x0000000102d03477 > 22:02:05 INFO - rsp = 0x0000000149bfec50 rbp = 0x0000000149bfec60 > 22:02:05 INFO - Found by: call frame info > 22:02:05 INFO - 2 XUL!mozilla::dom::EventBinding::preventDefault [EventBinding.cpp:6735250b8900 : 383 + 0xa] > 22:02:05 INFO - rbx = 0x0000000149bfec90 r12 = 0x000000011936af00 > 22:02:05 INFO - r13 = 0x0000000149bff53a r14 = 0x000000011936afe0 > 22:02:05 INFO - r15 = 0x000000010224f400 rip = 0x000000010224f987 > 22:02:05 INFO - rsp = 0x0000000149bfec70 rbp = 0x0000000149bfec80 > 22:02:05 INFO - Found by: call frame info > 22:02:05 INFO - 3 XUL!mozilla::dom::EventBinding::genericMethod [EventBinding.cpp:6735250b8900 : 663 + 0x4] > 22:02:05 INFO - rbx = 0x0000000125ba4098 r12 = 0x000000011936af00 > 22:02:05 INFO - r13 = 0x0000000149bff53a r14 = 0x000000011936afe0 > 22:02:05 INFO - r15 = 0x000000010224f400 rip = 0x000000010224f4dd > 22:02:05 INFO - rsp = 0x0000000149bfec90 rbp = 0x0000000149bfecf0 > 22:02:05 INFO - Found by: call frame info > 22:02:05 INFO - 4 XUL!js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) [jscntxtinlines.h:6735250b8900 : 220 + 0x5] > 22:02:05 INFO - rbx = 0x000000011936afe0 r12 = 0x000000011936af00 > 22:02:05 INFO - r13 = 0x0000000149bff53a r14 = 0x0000000149bff170 > 22:02:05 INFO - r15 = 0x000000010224f400 rip = 0x0000000104301541 > 22:02:05 INFO - rsp = 0x0000000149bfed00 rbp = 0x0000000149bfed30 > 22:02:05 INFO - Found by: call frame info Any ideas?

The value is modified here: http://mxr.mozilla.org/mozilla-central/source/dom/workers/Principal.cpp#22

Oh right, that is main thread only. workers are simpler. mIsMainThreadEvent ? xpc::AccessCheck::isChrome(js::GetContextCompartment(cx)) : mozilla::dom::workers::IsCurrentThreadRunningChromeWorker();

Okay, this works fine! https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=d2a6f07aaf57

(In reply to Olli Pettay [:smaug] from comment #20) > Comment on attachment 825349 [details] [diff] [review] > part.2 Fix test_assign_event_data.html for new defaultPrevented behavior and > make it a chrome test > > I don't understand the changes to runNextTest(), It tries the all same tests again but the event handler calls preventDefault(). So, the additional cases test Assign*EventData() with default prevented state. > and the timeout is scary. > refresh driver may not run within 100ms if we happen to run the test > on a debug build and GC/CC runs after the setTimeout call. > > If you need refresh driver checks, use requestAnimationFrame Thank you, it works!

Comment on attachment 8342320 [details] [diff] [review] part.1 Event.defaultPrevented should be false even if preventDefault() has been called by default action handler >+bool >+nsDOMEvent::IsChromeContext(JSContext* aCx) const Since the context isn't actually chrome (but the compartment), could you rename to IsChrome(JSContext* aCx) >+ // If this has already been consumed by non-default handler, >+ // we shouldn't update mDefaultPreventedByDefaultHandler. >+ if (!(mEvent->mFlags.mDefaultPrevented && >+ !mEvent->mFlags.mDefaultPreventedByDefaultHandler)) { I don't understand !mEvent->mFlags.mDefaultPreventedByDefaultHandler I'd expect if (aCalledByDefaultHandler && !mEvent->mFlags.mDefaultPrevented) { mEvent->mFlags.mDefaultPreventedByDefaultHandler = true; >+ // Need to set an extra flag for drag events. >+ if (mEvent->eventStructType == NS_DRAG_EVENT && IsTrusted()) { >+ nsCOMPtr<nsINode> node = do_QueryInterface(mEvent->currentTarget); >+ if (!node) { >+ nsCOMPtr<nsPIDOMWindow> win = do_QueryInterface(mEvent->currentTarget); >+ if (win) { >+ node = win->GetExtantDoc(); > } > } >+ if (node && !nsContentUtils::IsChromeDoc(node->OwnerDoc())) { >+ mEvent->mFlags.mDefaultPreventedByContent = true; >+ } We could probably probably simplify this by just using the information about aCalledByDefaultHandler. But let's change that in a followup. Please explain or fix >+ if (!(mEvent->mFlags.mDefaultPrevented && >+ !mEvent->mFlags.mDefaultPreventedByDefaultHandler)) {

(In reply to Olli Pettay [:smaug] from comment #29) > Comment on attachment 8342320 [details] [diff] [review] > part.1 Event.defaultPrevented should be false even if preventDefault() has > been called by default action handler > >+ // If this has already been consumed by non-default handler, > >+ // we shouldn't update mDefaultPreventedByDefaultHandler. > >+ if (!(mEvent->mFlags.mDefaultPrevented && > >+ !mEvent->mFlags.mDefaultPreventedByDefaultHandler)) { > I don't understand !mEvent->mFlags.mDefaultPreventedByDefaultHandler > I'd expect > if (aCalledByDefaultHandler && > !mEvent->mFlags.mDefaultPrevented) { > mEvent->mFlags.mDefaultPreventedByDefaultHandler = true; No, if we do so, following case fails: 1. Chrome such as UI or Add-on call preventDefault(), then, both mDefaultPrevented and mDefaultPreventedByDefaultHandler are true. 2. Content such as Web apps call preventDefault(), then, mDefaultPreventedByDefaultHandler is still true. This is odd if content checks defaultPrevented attribute. > >+ // Need to set an extra flag for drag events. > >+ if (mEvent->eventStructType == NS_DRAG_EVENT && IsTrusted()) { > >+ nsCOMPtr<nsINode> node = do_QueryInterface(mEvent->currentTarget); > >+ if (!node) { > >+ nsCOMPtr<nsPIDOMWindow> win = do_QueryInterface(mEvent->currentTarget); > >+ if (win) { > >+ node = win->GetExtantDoc(); > > } > > } > >+ if (node && !nsContentUtils::IsChromeDoc(node->OwnerDoc())) { > >+ mEvent->mFlags.mDefaultPreventedByContent = true; > >+ } > We could probably probably simplify this by just using the information about > aCalledByDefaultHandler. But let's change that in a followup. Agreed. > Please explain or fix > >+ if (!(mEvent->mFlags.mDefaultPrevented && > >+ !mEvent->mFlags.mDefaultPreventedByDefaultHandler)) { Okay, I'll explain it by comment.

Comment on attachment 8343439 [details] [diff] [review] part.1 Event.defaultPrevented should be false even if preventDefault() has been called by default action handler >+ // In following cases, we need to update mDefaultPreventedByDefaultHandler: >+ // 1. PreventDefault() hasn't been called yet. >+ // 2. PreventDefault() has already been called by chrome, but it's also called >+ // again by content because if we don't mDefaultPreventedByDefaultHandler >+ // in this case, Event.defaultPrevented becomes false for content even >+ // though it called Event.preventDefault(). >+ // Please don't update mDefaultPreventedByDefaultHandler when PreventDefault() >+ // has already called by content and it's called again by chrome. If we >+ // update it to true, Event.defaultPrevented becomes false even though content >+ // called Event.preventDefault(). Would it be simpler to use mDefaultPreventedByContent effectively mEvent->mFlags.mDefaultPrevented = true; if (!aCalledByDefaultHandler) { mEvent->mFlags.mDefaultPreventedByContent = true; } >+nsDOMEvent::DefaultPrevented(JSContext* aCx) const >+{ ... >+ >+ // If preventDefault() has been called by content, just return true. >+ if (!mEvent->mFlags.mDefaultPreventedByDefaultHandler) { >+ return true; >+ } >+ >+ // Otherwise, change the result for each caller. If caller of this is content, >+ // defaultPrevented attribute should be false. Otherwise, i.e., caller of >+ // this is chrome, defaultPrevented should be true. >+ return IsChrome(aCx); >+} Then replace the stuff above with return mEvent->mFlags.mDefaultPreventedByContent || IsChrome(aCx);

And yes, that would require to remove that drag event special case.

Hmm, I worry about the risk of using mDefaultPreventedByContent in this schedule. As I cced you to Jonathan's email, he needs this fix ASAP. Are you sure that it's safe to modify mDefaultPreventedByContent from all events??

Could that change be a follow-up bug?

Using mDefaultPreventedByContent should be safe. It is used currently only in one place in ESM.

Dispatching legacy mouse events which inherit defaultPrevented state needs to copy mDefaultPreventedByContent too. And it cannot be represented by nsEventStatus. So, I think that it is time to refactor nsEventStatus in follow up bug.

Comment on attachment 8344272 [details] [diff] [review] part.1 Event.defaultPrevented should be false even if preventDefault() has been called by default action handler > * @param aTargetFrame The event target of wheel event. > * @param aEvent The original Wheel event. >- * @param aStatus The event status, must not be >- * nsEventStatus_eConsumeNoDefault. >+ * @param aState The event which should be set to the dispatching >+ * event. This also returns the dispatched event >+ * state. perhaps inout event state to pass the old state and get new state.

And yes, it would be great to get rid of nsEventStatus. In a followup.

remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/c16b9aeb5de9 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/60c68bfe126e remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/97bf81b22f1f remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/2aaff66026ba

Thank you both for you expedited work on this! Much appreciated!

I have to say that I found the description of this bug difficult to read. Would it be fair for me to summarise it as follows: 1. When content accesses defaultPrevented, it only returns true if content called preventDefault() on the event. 2. When chrome or C++ accesses defaultPrevented, it returns true if anyone called preventDefault() on the event (in the case of C++ this refers to the version without the JS context). 3. When anyone access getPreventDefault(), it returns true if anyone called preventDefault() on the event (but this method is deprecated).

Exactlly.

https://hg.mozilla.org/mozilla-central/rev/c16b9aeb5de9 https://hg.mozilla.org/mozilla-central/rev/60c68bfe126e https://hg.mozilla.org/mozilla-central/rev/97bf81b22f1f https://hg.mozilla.org/mozilla-central/rev/2aaff66026ba