From 7b51dc72b75ecef83bcbee989ae4865f67ce1287 Mon Sep 17 00:00:00 2001 From: Ray Kraesig Date: Wed, 22 Feb 2023 21:05:24 +0000 Subject: [PATCH] Bug 1816740 [5/6] - convert nsIFilePicker::capture* to a proper enum r=ipc-reviewers,karlt,mstange,handyman,nika Convert the various capture* constants in nsIFilePicker into a proper enum, and perform validation when passing it across IPC. Additionally, unlike the previous two enums, reduce the size of CaptureTarget to 8 bits. This is a workaround for its use with nsAttrValue, which at present very specifically requires that parseable enums' values fit within an `int16_t` -- and a `uint16_t` does not. Differential Revision: https://phabricator.services.mozilla.com/D169854 --- dom/html/HTMLInputElement.cpp | 11 ++++++----- dom/ipc/FilePickerMessageUtils.h | 7 +++++++ dom/ipc/FilePickerParent.cpp | 2 +- dom/ipc/FilePickerParent.h | 2 +- dom/ipc/PFilePicker.ipdl | 3 ++- widget/nsBaseFilePicker.cpp | 10 +++++++--- widget/nsBaseFilePicker.h | 4 ++-- widget/nsFilePickerProxy.cpp | 4 ++-- widget/nsFilePickerProxy.h | 6 +++--- widget/nsIFilePicker.idl | 17 ++++++++++++----- 10 files changed, 43 insertions(+), 23 deletions(-) diff --git a/dom/html/HTMLInputElement.cpp b/dom/html/HTMLInputElement.cpp index 15f82818986d..cb486a7b1270 100644 --- a/dom/html/HTMLInputElement.cpp +++ b/dom/html/HTMLInputElement.cpp @@ -181,10 +181,10 @@ static const nsAttrValue::EnumTable* kInputDefaultType = &kInputTypeTable[ArrayLength(kInputTypeTable) - 2]; static const nsAttrValue::EnumTable kCaptureTable[] = { - {"user", static_cast(nsIFilePicker::captureUser)}, - {"environment", static_cast(nsIFilePicker::captureEnv)}, - {"", static_cast(nsIFilePicker::captureDefault)}, - {nullptr, static_cast(nsIFilePicker::captureNone)}}; + {"user", nsIFilePicker::captureUser}, + {"environment", nsIFilePicker::captureEnv}, + {"", nsIFilePicker::captureDefault}, + {nullptr, nsIFilePicker::captureNone}}; static const nsAttrValue::EnumTable* kCaptureDefault = &kCaptureTable[2]; @@ -831,7 +831,8 @@ nsresult HTMLInputElement::InitFilePicker(FilePickerType aType) { const nsAttrValue* captureVal = GetParsedAttr(nsGkAtoms::capture, kNameSpaceID_None); if (captureVal) { - filePicker->SetCapture(captureVal->GetEnumValue()); + filePicker->SetCapture(static_cast( + captureVal->GetEnumValue())); } } } else { diff --git a/dom/ipc/FilePickerMessageUtils.h b/dom/ipc/FilePickerMessageUtils.h index 857da64d940c..a77329906e46 100644 --- a/dom/ipc/FilePickerMessageUtils.h +++ b/dom/ipc/FilePickerMessageUtils.h @@ -17,6 +17,13 @@ struct ParamTraits nsIFilePicker::Mode, nsIFilePicker::Mode::modeOpen, nsIFilePicker::Mode::modeGetFolder> {}; +template <> +struct ParamTraits + : public ContiguousEnumSerializerInclusive< + nsIFilePicker::CaptureTarget, + nsIFilePicker::CaptureTarget::captureNone, + nsIFilePicker::CaptureTarget::captureEnv> {}; + template <> struct ParamTraits : public ContiguousEnumSerializerInclusive< diff --git a/dom/ipc/FilePickerParent.cpp b/dom/ipc/FilePickerParent.cpp index 0db81e0487cc..0817a25a72b9 100644 --- a/dom/ipc/FilePickerParent.cpp +++ b/dom/ipc/FilePickerParent.cpp @@ -242,7 +242,7 @@ mozilla::ipc::IPCResult FilePickerParent::RecvOpen( nsTArray&& aFilters, nsTArray&& aFilterNames, nsTArray&& aRawFilters, const nsString& aDisplayDirectory, const nsString& aDisplaySpecialDirectory, const nsString& aOkButtonLabel, - const int16_t& aCapture) { + const nsIFilePicker::CaptureTarget& aCapture) { if (!CreateFilePicker()) { Unused << Send__delete__(this, void_t(), nsIFilePicker::returnCancel); return IPC_OK(); diff --git a/dom/ipc/FilePickerParent.h b/dom/ipc/FilePickerParent.h index 45794fea2e70..67ce7878c493 100644 --- a/dom/ipc/FilePickerParent.h +++ b/dom/ipc/FilePickerParent.h @@ -42,7 +42,7 @@ class FilePickerParent : public PFilePickerParent { nsTArray&& aFilters, nsTArray&& aFilterNames, nsTArray&& aRawFilters, const nsString& aDisplayDirectory, const nsString& aDisplaySpecialDirectory, const nsString& aOkButtonLabel, - const int16_t& aCapture); + const nsIFilePicker::CaptureTarget& aCapture); virtual void ActorDestroy(ActorDestroyReason aWhy) override; diff --git a/dom/ipc/PFilePicker.ipdl b/dom/ipc/PFilePicker.ipdl index 9fd7b8f91790..b245a2f9ea90 100644 --- a/dom/ipc/PFilePicker.ipdl +++ b/dom/ipc/PFilePicker.ipdl @@ -11,6 +11,7 @@ include IPCBlob; include "mozilla/dom/FilePickerMessageUtils.h"; using struct mozilla::void_t from "mozilla/ipc/IPCCore.h"; +using nsIFilePicker::CaptureTarget from "nsIFilePicker.h"; using nsIFilePicker::ResultCode from "nsIFilePicker.h"; namespace mozilla { @@ -43,7 +44,7 @@ parent: nsString defaultExtension, nsString[] filters, nsString[] filterNames, nsString[] rawFilters, nsString displayDirectory, nsString displaySpecialDirectory, nsString okButtonLabel, - int16_t capture); + CaptureTarget capture); child: async __delete__(MaybeInputData data, ResultCode result); diff --git a/widget/nsBaseFilePicker.cpp b/widget/nsBaseFilePicker.cpp index 54ab12832864..c7b0b5007589 100644 --- a/widget/nsBaseFilePicker.cpp +++ b/widget/nsBaseFilePicker.cpp @@ -245,12 +245,16 @@ NS_IMETHODIMP nsBaseFilePicker::AppendRawFilter(const nsAString& aFilter) { return NS_OK; } -NS_IMETHODIMP nsBaseFilePicker::GetCapture(int16_t* aCapture) { - *aCapture = 0; +NS_IMETHODIMP nsBaseFilePicker::GetCapture( + nsIFilePicker::CaptureTarget* aCapture) { + *aCapture = nsIFilePicker::CaptureTarget::captureNone; return NS_OK; } -NS_IMETHODIMP nsBaseFilePicker::SetCapture(int16_t aCapture) { return NS_OK; } +NS_IMETHODIMP nsBaseFilePicker::SetCapture( + nsIFilePicker::CaptureTarget aCapture) { + return NS_OK; +} // Set the filter index NS_IMETHODIMP nsBaseFilePicker::GetFilterIndex(int32_t* aFilterIndex) { diff --git a/widget/nsBaseFilePicker.h b/widget/nsBaseFilePicker.h index 7bafcab92099..b7274508d7da 100644 --- a/widget/nsBaseFilePicker.h +++ b/widget/nsBaseFilePicker.h @@ -31,8 +31,8 @@ class nsBaseFilePicker : public nsIFilePicker { NS_IMETHOD Open(nsIFilePickerShownCallback* aCallback) override; NS_IMETHOD AppendFilters(int32_t filterMask) override; NS_IMETHOD AppendRawFilter(const nsAString& aFilter) override; - NS_IMETHOD GetCapture(int16_t* aCapture) override; - NS_IMETHOD SetCapture(int16_t aCapture) override; + NS_IMETHOD GetCapture(nsIFilePicker::CaptureTarget* aCapture) override; + NS_IMETHOD SetCapture(nsIFilePicker::CaptureTarget aCapture) override; NS_IMETHOD GetFilterIndex(int32_t* aFilterIndex) override; NS_IMETHOD SetFilterIndex(int32_t aFilterIndex) override; NS_IMETHOD GetFiles(nsISimpleEnumerator** aFiles) override; diff --git a/widget/nsFilePickerProxy.cpp b/widget/nsFilePickerProxy.cpp index 0d7c15c6ee1d..542b7bf0ff2c 100644 --- a/widget/nsFilePickerProxy.cpp +++ b/widget/nsFilePickerProxy.cpp @@ -54,13 +54,13 @@ nsFilePickerProxy::AppendFilter(const nsAString& aTitle, } NS_IMETHODIMP -nsFilePickerProxy::GetCapture(int16_t* aCapture) { +nsFilePickerProxy::GetCapture(nsIFilePicker::CaptureTarget* aCapture) { *aCapture = mCapture; return NS_OK; } NS_IMETHODIMP -nsFilePickerProxy::SetCapture(int16_t aCapture) { +nsFilePickerProxy::SetCapture(nsIFilePicker::CaptureTarget aCapture) { mCapture = aCapture; return NS_OK; } diff --git a/widget/nsFilePickerProxy.h b/widget/nsFilePickerProxy.h index a4c4654efbce..4644d26bff9f 100644 --- a/widget/nsFilePickerProxy.h +++ b/widget/nsFilePickerProxy.h @@ -37,8 +37,8 @@ class nsFilePickerProxy : public nsBaseFilePicker, nsIFilePicker::Mode aMode) override; NS_IMETHOD AppendFilter(const nsAString& aTitle, const nsAString& aFilter) override; - NS_IMETHOD GetCapture(int16_t* aCapture) override; - NS_IMETHOD SetCapture(int16_t aCapture) override; + NS_IMETHOD GetCapture(nsIFilePicker::CaptureTarget* aCapture) override; + NS_IMETHOD SetCapture(nsIFilePicker::CaptureTarget aCapture) override; NS_IMETHOD GetDefaultString(nsAString& aDefaultString) override; NS_IMETHOD SetDefaultString(const nsAString& aDefaultString) override; NS_IMETHOD GetDefaultExtension(nsAString& aDefaultExtension) override; @@ -74,7 +74,7 @@ class nsFilePickerProxy : public nsBaseFilePicker, nsString mFile; nsString mDefault; nsString mDefaultExtension; - int16_t mCapture; + nsIFilePicker::CaptureTarget mCapture; bool mIPCActive; diff --git a/widget/nsIFilePicker.idl b/widget/nsIFilePicker.idl index 034e3f9ceb6c..5e625d1ff28d 100644 --- a/widget/nsIFilePicker.idl +++ b/widget/nsIFilePicker.idl @@ -53,10 +53,12 @@ interface nsIFilePicker : nsISupports // *.rm; *.rmvb; *.smil; *.webm; // *.wmv; *.xvid - const short captureNone = 0; // No capture target specified. - const short captureDefault = 1; // Missing/invalid value default. - const short captureUser = 2; // "user" capture target specified. - const short captureEnv = 3; // "environment" capture target specified. + cenum CaptureTarget: 8 { + captureNone = 0, // No capture target specified. + captureDefault = 1, // Missing/invalid value default. + captureUser = 2, // "user" capture target specified. + captureEnv = 3, // "environment" capture target specified. + }; /** * Initialize the file picker widget. The file picker is not valid until this @@ -206,7 +208,12 @@ interface nsIFilePicker : nsISupports */ attribute AString okButtonLabel; - attribute short capture; + /** + * Implementation of HTMLInputElement's `capture` property. + * + * Not used by Firefox Desktop. + */ + attribute nsIFilePicker_CaptureTarget capture; }; [scriptable, function, uuid(0d79adad-b244-49A5-9997-2a8cad93fc44)]