724161 - simplify drag&drop checks for javascript:/data: URIs

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[:Gavin Sharp [email: [e-mail]]]

The patch for bug 718203 exposes a nicer API for filtering out javascript:/data: link drops, we should make use of that rather than having different string-comparison checks.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch

with tests

Comment 3

[Dão Gottwald [:dao]]

Comment on attachment 597203 [details] [diff] [review]
patch

>+++ b/browser/base/content/test/browser_tabDrop.js

>+      chromeUtils.synthesizeDrop(newTab, newTab, [[{type: "text/plain", data: text}]], "link", window, EventUtils);

I would expect this to load the links in the current tab, but you're expecting new tabs. What's going on here?

Comment 4

[:Gavin Sharp [email: [e-mail]]]

(In reply to Dão Gottwald [:dao] from comment #3)
> I would expect this to load the links in the current tab, but you're
> expecting new tabs. What's going on here?

Good catch. The synthesizeDrop code is producing events with screenX/screenY equal to 0, which makes _getDragTargetTab return null.

Fixing synthesizeDrop to pass realistic coordinates seems like it might be annoying. What do you think about just adding a comment explaining the reliance on the bug?

Comment 5

[Dão Gottwald [:dao]]

Could the synthetic drop target the left or right side of the tab so that it keeps working in case that bug gets fixed?

Comment 6

[:Gavin Sharp [email: [e-mail]]]

(In reply to Dão Gottwald [:dao] from comment #5)
> Could the synthetic drop target the left or right side of the tab so that it
> keeps working in case that bug gets fixed?

There's no way to control the specific coordinates of the drop. I think anyone fixing synthesizeDrop to pass non-bogus coordinates will also have to add support for specifying custom offsets to avoid breaking the world, so it makes more sense to just deal with this test when (if) that happens.

Comment 7

[:Gavin Sharp [email: [e-mail]]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/50aae34555bb

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/50aae34555bb

Comment 9

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 597203 [details] [diff] [review]
patch

[Approval Request Comment]
fix for bug 704354. depends on patch for bug 718203.

Comment 10

[Alex Keybl [:akeybl]]

Comment on attachment 597203 [details] [diff] [review]
patch

[Triage Comment]
Approving for Aurora/Beta in support of bug 704354. Please land today.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

https://hg.mozilla.org/releases/mozilla-beta/rev/dbbd683b1c87
https://hg.mozilla.org/releases/mozilla-aurora/rev/b740b9528028

Comment 12

[:Gavin Sharp [email: [e-mail]]]

https://hg.mozilla.org/releases/mozilla-esr10/rev/566be745f397