Closed Bug 980134 Opened 8 years ago Closed 8 years ago

Hide DataContainerEvent from content

Categories

(Core :: DOM: Events, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: emk, Assigned: emk)

References

Details

(Keywords: addon-compat, dev-doc-complete, site-compat)

Attachments

(1 file, 4 obsolete files)

This is a Firefox-specific extension. No MDN docs. Standard alternatives (MessageEvent and CustomEvent) are available.
Blocks: stdglobal
Alex, does a11y still use DataContainerEvent?
Perhaps we could just make is [ChromeOnly] and dispatch to chrome too?
document.createEvent("datacontainerevents") should behave as if such an event type is not present.
Aldo Thunderbird still depends on DataContainerEvent.
https://mxr.mozilla.org/comm-central/search?string=DataContainerEvent
I'll separate the chrome removal to another bug.
(In reply to Olli Pettay [:smaug] from comment #1)
> Alex, does a11y still use DataContainerEvent?

looks like it see accessible/src/generic/RootAccessible.cpp, that said I'd absolutely take a patch replacing it with something c++ only if someone wrote it.
Depends on: 980278
Attached patch patch (obsolete) — Splinter Review
Attachment #8386741 - Flags: review?(bugs)
Blocks: 980280
Comment on attachment 8386741 [details] [diff] [review]
patch

This doesn't prevent content JS to access these event during event propagation.

Could we remove support for document.createEvent("datacontainerevent");
and make the event to always have mOnlyChromeDispatch set true.
Like, set mEvent.mOnlyChromeDispatch = true; in DataContainerEvent.
Interface should be [ChromeOnly]

That does mean the test_bug368835.html should become a chrome test.
Attachment #8386741 - Flags: review?(bugs) → review-
right, it's used for XUL trees, direct layout call is prefereable
(In reply to Olli Pettay [:smaug] from comment #7)
> Could we remove support for document.createEvent("datacontainerevent");

How can test_bug368835.html create the event even if it becomes a chrome test?
Oh, right, the event doesn't have a ctor. I guess we could support document.createEvent in chrome context.
Attached patch patch v2 (obsolete) — Splinter Review
Prevented the events from being dispatched to content.
Now test_bug368835.html will be executed from both chrome and content to make sure that DataContainerEvent will not be dispatched to content.
Attachment #8386741 - Attachment is obsolete: true
Attachment #8386924 - Flags: review?(bugs)
Comment on attachment 8386924 [details] [diff] [review]
patch v2


> NS_NewDOMDataContainerEvent(nsIDOMEvent** aInstancePtrResult,
>                             EventTarget* aOwner,
>                             nsPresContext* aPresContext,
>                             WidgetEvent* aEvent)
> {
>+  if (!nsContentUtils::IsCallerChrome()) {
>+    return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
>+  }
So the issue is that we're possibly running content JS, but C++ ends up creating the event.
We need to make C++ to create the event explicitly and then in createEvent impl check whether the caller chrome.
Attachment #8386924 - Flags: review?(bugs) → review-
Depends on: 981237
Depends on: 980307
Attached patch patch v3 (obsolete) — Splinter Review
Move the privilege check into nsEventDispatcher::CreateEvent.
Once bug 981237 and bug 980307 are fixed, there is no in-tree user of DataContainerEvent anymore.
Attachment #8386924 - Attachment is obsolete: true
Attachment #8388033 - Flags: review?(bugs)
Comment on attachment 8388033 [details] [diff] [review]
patch v3

Could you please make C++ callers to not use CreateEvent().
Otherwise the notifications from nsTreeBodyFrame may not work.
Or remove those callers, and ask review for this later.
(And if we don't have any use for DataContainerEvent, we could just remove it.)
Attachment #8388033 - Flags: review?(bugs) → review-
(In reply to Olli Pettay [:smaug] from comment #14)
> Could you please make C++ callers to not use CreateEvent().
> Otherwise the notifications from nsTreeBodyFrame may not work.
> Or remove those callers, and ask review for this later.

OK, I'll request review again after bug 981237 and bug 980307.
(In reply to Olli Pettay [:smaug] from comment #14)
> (And if we don't have any use for DataContainerEvent, we could just remove
> it.)

Sorry, I should have said "there is no in-tree C++ user of DataContainerEvent anymore".
Moreover, some add-ons (e.g. Tree Style Tab and Multiple Tab Handler) are using this event, so I think we need a grace period.
Hmm, CustomEvent seems useful enough for me.
https://developer.mozilla.org/ja/docs/Web/API/customEvent
I'll migrate my codes. Thanks.
Depends on: 986756
Attached patch patch v4 (obsolete) — Splinter Review
I stopped to making this event chrome-dispatch-only because some existing third-party consumers are using this event as a communication channel between chrome and content.
I added a deprecation warning instead.
Attachment #8388033 - Attachment is obsolete: true
Attachment #8397061 - Flags: review?(bugs)
Attached patch patch v4.1Splinter Review
Fixed b2g build bustage
Attachment #8397061 - Attachment is obsolete: true
Attachment #8397061 - Flags: review?(bugs)
Attachment #8397900 - Flags: review?(bugs)
Comment on attachment 8397900 [details] [diff] [review]
patch v4.1

># HG changeset patch
># Parent 0a537bc94d34af680c262b19bed317bdc09e1144
># User Masatoshi Kimura <VYV03354@nifty.ne.jp>
>Bug 980134 - Hide DataContainerEvent from content. r?smaug
>
>diff --git a/content/base/public/nsDeprecatedOperationList.h b/content/base/public/nsDeprecatedOperationList.h
>--- a/content/base/public/nsDeprecatedOperationList.h
>+++ b/content/base/public/nsDeprecatedOperationList.h
>@@ -34,8 +34,9 @@ DEPRECATED_OPERATION(GetPreventDefault)
> DEPRECATED_OPERATION(GetSetUserData)
> DEPRECATED_OPERATION(MozGetAsFile)
> DEPRECATED_OPERATION(UseOfCaptureEvents)
> DEPRECATED_OPERATION(UseOfReleaseEvents)
> DEPRECATED_OPERATION(UseOfDOM3LoadMethod)
> DEPRECATED_OPERATION(ShowModalDialog)
> DEPRECATED_OPERATION(Window_Content)
> DEPRECATED_OPERATION(SyncXMLHttpRequest)
>+DEPRECATED_OPERATION(DataContainerEvent)
>diff --git a/dom/events/DataContainerEvent.cpp b/dom/events/DataContainerEvent.cpp
>--- a/dom/events/DataContainerEvent.cpp
>+++ b/dom/events/DataContainerEvent.cpp
>@@ -1,25 +1,31 @@
> /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
> /* This Source Code Form is subject to the terms of the Mozilla Public
>  * License, v. 2.0. If a copy of the MPL was not distributed with this
>  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> 
> #include "mozilla/dom/DataContainerEvent.h"
> #include "nsContentUtils.h"
>+#include "nsIDocument.h"
> #include "nsIXPConnect.h"
> 
> namespace mozilla {
> namespace dom {
> 
> DataContainerEvent::DataContainerEvent(EventTarget* aOwner,
>                                        nsPresContext* aPresContext,
>                                        WidgetEvent* aEvent)
>   : Event(aOwner, aPresContext, aEvent)
> {
>+  if (mOwner) {
>+    if (nsIDocument* doc = mOwner->GetExtantDoc()) {
>+      doc->WarnOnceAbout(nsIDocument::eDataContainerEvent);
>+    }
>+  }
> }
> 
> NS_IMPL_CYCLE_COLLECTION_CLASS(DataContainerEvent)
> 
> NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(DataContainerEvent, Event)
>   tmp->mData.Clear();
> NS_IMPL_CYCLE_COLLECTION_UNLINK_END
> 
>diff --git a/dom/events/test/chrome.ini b/dom/events/test/chrome.ini
>--- a/dom/events/test/chrome.ini
>+++ b/dom/events/test/chrome.ini
>@@ -4,16 +4,17 @@ support-files =
>   bug415498-doc2.html
>   bug591249_iframe.xul
>   bug602962.xul
>   file_bug679494.html
>   window_bug617528.xul
> 
> [test_bug336682.js]
> [test_bug336682_2.xul]
>+[test_bug368835.html]
> [test_bug415498.xul]
> [test_bug586961.xul]
> [test_bug591249.xul]
> [test_bug602962.xul]
> [test_bug617528.xul]
> [test_bug679494.xul]
> [test_bug930374-chrome.html]
> [test_eventctors.xul]
>diff --git a/dom/events/test/test_bug368835.html b/dom/events/test/test_bug368835.html
>--- a/dom/events/test/test_bug368835.html
>+++ b/dom/events/test/test_bug368835.html
>@@ -1,19 +1,19 @@
> <!DOCTYPE HTML>
> <html>
> <!--
> https://bugzilla.mozilla.org/show_bug.cgi?id=368835
> -->
>   <head>
>     <title>Test for Bug 368835</title>
> 
>-    <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
>+    <link rel="stylesheet" type="text/css" href="http://mochi.test:8888/tests/SimpleTest/test.css" />
> 
>-    <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
>+    <script type="text/javascript" src="http://mochi.test:8888/tests/SimpleTest/SimpleTest.js"></script>
>   </head>
> 
>   <body>
>     <a target="_blank"
>        href="https://bugzilla.mozilla.org/show_bug.cgi?id=368835">
>       Mozilla Bug 368835
>     </a>
>     <p id="display"></p>
>@@ -84,18 +84,18 @@ https://bugzilla.mozilla.org/show_bug.cg
>           isPassed = false;
>         }
> 
>         ok(isPassed, "setData shouldn't fail when event is initialized.");
> 
>         document.body.addEventListener("dataContainerEvent",
>                                        dataContainerEventHandler, true);
>         document.body.dispatchEvent(event);
>+        SimpleTest.finish();
>       }
> 
>       SimpleTest.waitForExplicitFinish();
>       addLoadEvent(doTest);
>-      addLoadEvent(SimpleTest.finish);
>     </script>
>     </pre>
>   </body>
> </html>
> 
>diff --git a/dom/locales/en-US/chrome/dom/dom.properties b/dom/locales/en-US/chrome/dom/dom.properties
>--- a/dom/locales/en-US/chrome/dom/dom.properties
>+++ b/dom/locales/en-US/chrome/dom/dom.properties
>@@ -141,8 +141,10 @@ UseOfReleaseEventsWarning=Use of release
> UseOfDOM3LoadMethodWarning=Use of document.load() is deprecated. To upgrade your code, use the DOM XMLHttpRequest object. For more help https://developer.mozilla.org/en/XMLHttpRequest
> # LOCALIZATION NOTE: Do not translate "window.showModalDialog()" or "window.open()" 
> ShowModalDialogWarning=Use of window.showModalDialog() is deprecated. Use window.open() instead. For more help https://developer.mozilla.org/en-US/docs/Web/API/Window.open
> # LOCALIZATION NOTE: Do not translate "window._content" or "window.content"
> Window_ContentWarning=window._content is deprecated.  Please use window.content instead.
> # LOCALIZATION NOTE: Do not translate "XMLHttpRequest"
> SyncXMLHttpRequestWarning=Synchronous XMLHttpRequest on the main thread is deprecated because of its detrimental effects to the end user's experience. For more help http://xhr.spec.whatwg.org/
> ImplicitMetaViewportTagFallback=No meta-viewport tag found. Please explicitly specify one to prevent unexpected behavioural changes in future versions. For more help https://developer.mozilla.org/en/docs/Mozilla/Mobile/Viewport_meta_tag
>+# LOCALIZATION NOTE: Do not translate "DataContainerEvent" or "CustomEvent"
>+DataContainerEventWarning=Use of DataContainerEvent is deprecated. Use CustomEvent instead.
>diff --git a/dom/tests/mochitest/general/test_interfaces.html b/dom/tests/mochitest/general/test_interfaces.html
>--- a/dom/tests/mochitest/general/test_interfaces.html
>+++ b/dom/tests/mochitest/general/test_interfaces.html
>@@ -251,18 +251,16 @@ var interfaceNamesInGlobalScope =
>     "CSSValue",
> // IMPORTANT: Do not change this list without review from a DOM peer!
>     "CSSValueList",
> // IMPORTANT: Do not change this list without review from a DOM peer!
>     "CustomEvent",
> // IMPORTANT: Do not change this list without review from a DOM peer!
>     "DataChannel",
> // IMPORTANT: Do not change this list without review from a DOM peer!
>-    "DataContainerEvent",
>-// IMPORTANT: Do not change this list without review from a DOM peer!
>     {name: "DataErrorEvent", b2g: true, pref: "dom.mobileconnection.enabled"},
> // IMPORTANT: Do not change this list without review from a DOM peer!
>     {name: "DataStore", b2g: true},
> // IMPORTANT: Do not change this list without review from a DOM peer!
>     {name: "DataStoreChangeEvent", b2g: true},
> // IMPORTANT: Do not change this list without review from a DOM peer!
>     {name: "DataStoreCursor", b2g: true},
> // IMPORTANT: Do not change this list without review from a DOM peer!
>diff --git a/dom/webidl/DataContainerEvent.webidl b/dom/webidl/DataContainerEvent.webidl
>--- a/dom/webidl/DataContainerEvent.webidl
>+++ b/dom/webidl/DataContainerEvent.webidl
>@@ -1,16 +1,17 @@
> /* -*- Mode: IDL; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
> /* This Source Code Form is subject to the terms of the Mozilla Public
>  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
>  * You can obtain one at http://mozilla.org/MPL/2.0/.
>  */
> 
> interface nsIVariant;
> 
>+[ChromeOnly]
> interface DataContainerEvent : Event {
>   /**
>    * Return the data associated with the given key.
>    *
>    * @param  key  the key
>    * @return      the data associated with the key
>    */
>   nsIVariant? getData(DOMString? key);
>diff --git a/webapprt/locales/en-US/webapprt/overrides/dom.properties b/webapprt/locales/en-US/webapprt/overrides/dom.properties
>--- a/webapprt/locales/en-US/webapprt/overrides/dom.properties
>+++ b/webapprt/locales/en-US/webapprt/overrides/dom.properties
>@@ -140,8 +140,10 @@ UseOfReleaseEventsWarning=Use of release
> # LOCALIZATION NOTE: Do not translate "document.load()" or "XMLHttpRequest"
> UseOfDOM3LoadMethodWarning=Use of document.load() is deprecated. To upgrade your code, use the DOM XMLHttpRequest object. For more help https://developer.mozilla.org/en/XMLHttpRequest
> # LOCALIZATION NOTE: Do not translate "window.showModalDialog()" or "window.open()" 
> ShowModalDialogWarning=Use of window.showModalDialog() is deprecated. Use window.open() instead. For more help https://developer.mozilla.org/en-US/docs/Web/API/Window.open
> # LOCALIZATION NOTE: Do not translate "window._content" or "window.content"
> Window_ContentWarning=window._content is deprecated. Please use window.content instead.
> # LOCALIZATION NOTE: Do not translate "XMLHttpRequest"
> SyncXMLHttpRequestWarning=Synchronous XMLHttpRequest on the main thread is deprecated because of its detrimental effects to the end user's experience. For more help http://xhr.spec.whatwg.org/
>+# LOCALIZATION NOTE: Do not translate "DataContainerEvent" or "CustomEvent"
>+DataContainerEventWarning=Use of DataContainerEvent is deprecated. Use CustomEvent instead.
Attachment #8397900 - Flags: review?(bugs) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/3d2c3cba76af
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
Flags: in-testsuite+
Keywords: addon-compat
https://hg.mozilla.org/mozilla-central/rev/3d2c3cba76af
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.