Remove wrapping of nsISupports in transferables

RESOLVED FIXED in Firefox 64

Status

()

defect
P3
normal
RESOLVED FIXED
3 years ago
8 months ago

People

(Reporter: enndeakin, Assigned: evilpie)

Tracking

(Blocks 1 bug)

unspecified
mozilla64
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 wontfix, firefox57 wontfix, firefox58 wontfix, firefox59 ?, firefox64 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

3 years ago
Various callers of nsITransferable::SetData will wrap nsISupports in a nsISupportsInterfacePointer, but this shouldn't be needed. Remove these.
Reporter

Comment 1

3 years ago
Posted patch Patch that seems to work (obsolete) — Splinter Review
Reporter

Updated

2 years ago
Priority: -- → P3
Assignee

Comment 3

8 months ago
This seems like a good idea.
Flags: needinfo?(enndeakin)
Assignee

Updated

8 months ago
Blocks: 1493292
Assignee

Updated

8 months ago
Assignee: nobody → evilpies
No longer blocks: 1493292
Flags: needinfo?(enndeakin)
Assignee

Comment 4

8 months ago
I rebased the patch. I suspect we might actually be able to completely remove nsISupportsInterfacePointer.
Assignee

Updated

8 months ago
Attachment #9015127 - Attachment description: Rebased → Remove nsISupportsInterfacePointer usage from nsITransferables
Assignee

Updated

8 months ago
Attachment #8812180 - Attachment is obsolete: true
Assignee

Comment 5

8 months ago
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.
Assignee

Comment 6

8 months ago
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+

Comment 8

8 months ago
Pushed by evilpies@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/15fe5851f113
Remove wrapping of nsISupports as nsISupportsInterfacePointer in transferables. r=smaug
Assignee

Updated

8 months ago
Blocks: 1493292

Comment 9

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/15fe5851f113
Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Thanks, all!  I'm pleased to see this simplified.
Assignee

Updated

8 months ago
Blocks: 1497830
You need to log in before you can comment on or make changes to this bug.