Closed Bug 968643 Opened 6 years ago Closed 6 years ago

Remove the PrefControlled WebIDL annotation

Categories

(Core :: DOM: Core & HTML, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: bzbarsky, Assigned: ehsan)

References

Details

(Keywords: dev-doc-complete)

Attachments

(2 files, 2 obsolete files)

We can use Func instead, and reduce complexity.  Once bug 968479 is fixed...
Attachment #8371445 - Flags: review?(bzbarsky)
Keywords: dev-doc-needed
Assignee: nobody → ehsan
Attachment #8371445 - Attachment is obsolete: true
Attachment #8371445 - Flags: review?(bzbarsky)
Attachment #8371469 - Flags: review?(bzbarsky)
Attachment #8371470 - Flags: review?(bzbarsky)
Comment on attachment 8371469 [details] [diff] [review]
Part 1: Stop using [PrefControlled]; r=bzbarsky

>+  if (!nsDOMTouchEvent::PrefEnabled(nullptr, nullptr))

Could use default values in the decl of PrefEnabled to not require this change, right?

Same for EventSource.  And maybe file a followup to move EventSource to [Pref] and nix this extra check if it's not needed anymore?

And same for WebSocket.

>+ShadowRoot::PrefEnabled(JSContext* aCx, JSObject* aGlobal)

Can this just use a [Pref] annotation in the WebIDL instead and nix this function altogether?

>+UndoManager::PrefEnabled(JSContext* aCx, JSObject* aGlobal)

Again, can this just use a [Pref]?

>+EnableWebSpeechRecognitionCheck::PrefEnabled(JSContext* aCx, JSObject*

This seems like it could be a bunch of [Pref] annotations, but I guess that would duplicate the pref name in various files...  Let's not worry about it for now.

>+++ b/content/media/webspeech/recognition/EnableWebSpeechRecognitionCheck.h
>+#include "js/TypeDecls.h"

Can you get away with just reclaring the function to take void* and void*?  It's not like you use the arguments.  ;)

Same comments apply to EnableSpeechSynthesisCheck.

>+++ b/dom/activities/src/Activity.h

This should use [Pref] in the IDL.

>+++ b/dom/base/nsGlobalWindow.cpp
>+  if (SpeechSynthesis::PrefEnabled(nullptr, nullptr)) {

Again, default values for the win.

>+++ b/dom/events/nsEventDispatcher.cpp
>+      nsDOMTouchEvent::PrefEnabled(nullptr, nullptr))

And here.

>+++ b/dom/file/ArchiveReader.cpp
>+  MOZ_ASSERT(PrefEnabled(nullptr, nullptr));

We should just nix the assert, since it's actually totally bogus if someone changes the pref after loading the page.

>+++ b/dom/src/notification/Notification.cpp

Can this be a [Pref] instead?

>+++ b/dom/webidl/EventSource.webidl
>+[Constructor(DOMString url, optional EventSourceInit eventSourceInitDict),
>+Func="mozilla::dom::EventSource::PrefEnabled"]

Would prefer to line up the 'F' with the 'C', I think.

>+++ b/dom/webidl/MozActivity.webidl

Similar here.

>+++ b/dom/webidl/Notification.webidl

And here.

>+++ b/dom/webidl/SpeechGrammar.webidl

And here.

>+++ b/dom/webidl/SpeechRecognition.webidl

And here.

>+++ b/layout/style/CSS.h

This should just use a [Pref] in the IDL

r=me with those bits fixed, though I'd like to see the resulting patch.
Attachment #8371469 - Flags: review?(bzbarsky) → review+
Comment on attachment 8371470 [details] [diff] [review]
Part 2: Remove the [PrefControlled] WebIDL annotation; r=bzbarsky

r=me
Attachment #8371470 - Flags: review?(bzbarsky) → review+
I'll clean up the speech stuff in another bug (will use [Pref] and will get rid of these Enable* classes.)
Attachment #8371469 - Attachment is obsolete: true
Attachment #8371544 - Flags: review?(bzbarsky)
(In reply to comment #6)
> I'll clean up the speech stuff in another bug (will use [Pref] and will get rid
> of these Enable* classes.)

(Bug 968879)
Blocks: 968883
Comment on attachment 8371544 [details] [diff] [review]
Part 1: Stop using [PrefControlled]; r=bzbarsky

Should probably add a default value for the undomanager pref to all.js, because I'm not sure what default value Pref= uses.

The rest looks good.
Attachment #8371544 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/mozilla-central/rev/20a329b254ae
https://hg.mozilla.org/mozilla-central/rev/197880d71d9d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.