Closed Bug 1310193 Opened 4 years ago Closed 2 years ago

Remove wrapping of nsISupports in transferables

Categories

(Core :: DOM: Drag & Drop, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox56 --- wontfix
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- ?
firefox64 --- fixed

People

(Reporter: enndeakin, Assigned: evilpie)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

Various callers of nsITransferable::SetData will wrap nsISupports in a nsISupportsInterfacePointer, but this shouldn't be needed. Remove these.
Attached patch Patch that seems to work (obsolete) — Splinter Review
Priority: -- → P3
This seems like a good idea.
Flags: needinfo?(enndeakin)
Blocks: 1493292
Assignee: nobody → evilpies
No longer blocks: 1493292
Flags: needinfo?(enndeakin)
I rebased the patch. I suspect we might actually be able to completely remove nsISupportsInterfacePointer.
Attachment #9015127 - Attachment description: Rebased → Remove nsISupportsInterfacePointer usage from nsITransferables
Attachment #8812180 - Attachment is obsolete: true
Comment on attachment 9015127 [details] [diff] [review]
Remove nsISupportsInterfacePointer usage from nsITransferables

Review of attachment 9015127 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/extensions/parent/ext-clipboard.js
@@ +53,5 @@
>            // a nsIInputStream instance fails.
>            // For example HTMLEditor::InsertObject (HTMLEditorDataTransfer.cpp)
>            // and DataTransferItem::FillInExternalData (DataTransferItem.cpp).
>            //
>            // As a work-around, we force nsClipboard::GetNativeClipboardData to

We really need to remove this work-around if we want to remove the length parameter.
Comment on attachment 9015127 [details] [diff] [review]
Remove nsISupportsInterfacePointer usage from nsITransferables

Patch by Neil (will fix commit author)
Various callers of nsITransferable::SetData will wrap nsISupports in a nsISupportsInterfacePointer, that is not really necessary, we can just use the pointer directly. Most code actually already does that.
Attachment #9015127 - Flags: review?(bugs)
Comment on attachment 9015127 [details] [diff] [review]
Remove nsISupportsInterfacePointer usage from nsITransferables

I guess nsISupportsInterfacePointer was used originally to have some more consistency, but in practice it just makes code harder to follow and error prone.
Attachment #9015127 - Flags: review?(bugs) → review+
Pushed by evilpies@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/15fe5851f113
Remove wrapping of nsISupports as nsISupportsInterfacePointer in transferables. r=smaug
Blocks: 1493292
https://hg.mozilla.org/mozilla-central/rev/15fe5851f113
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Thanks, all!  I'm pleased to see this simplified.
Blocks: 1497830
You need to log in before you can comment on or make changes to this bug.