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

RESOLVED FIXED in Firefox 11

Status

()

Firefox
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Gavin, Assigned: Gavin)

Tracking

Trunk
Firefox 13
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox11 fixed, firefox12 fixed, firefox-esr10 fixed)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment, 1 obsolete attachment)

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.
Created attachment 594359 [details] [diff] [review]
patch
Assignee: nobody → gavin.sharp
Status: NEW → ASSIGNED
Blocks: 704354
Created attachment 597203 [details] [diff] [review]
patch

with tests
Attachment #594359 - Attachment is obsolete: true
Attachment #597203 - Flags: review?(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?
(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?
Could the synthetic drop target the left or right side of the tab so that it keeps working in case that bug gets fixed?
(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.

Updated

5 years ago
Attachment #597203 - Flags: review?(dao) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/50aae34555bb
Flags: in-testsuite+
Target Milestone: --- → Firefox 13

Comment 8

5 years ago
https://hg.mozilla.org/mozilla-central/rev/50aae34555bb
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Comment on attachment 597203 [details] [diff] [review]
patch

[Approval Request Comment]
fix for bug 704354. depends on patch for bug 718203.
Attachment #597203 - Flags: approval-mozilla-beta?
Attachment #597203 - Flags: approval-mozilla-aurora?
Comment on attachment 597203 [details] [diff] [review]
patch

[Triage Comment]
Approving for Aurora/Beta in support of bug 704354. Please land today.
Attachment #597203 - Flags: approval-mozilla-beta?
Attachment #597203 - Flags: approval-mozilla-beta+
Attachment #597203 - Flags: approval-mozilla-aurora?
Attachment #597203 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-beta/rev/dbbd683b1c87
https://hg.mozilla.org/releases/mozilla-aurora/rev/b740b9528028
status-firefox11: --- → fixed
status-firefox12: --- → fixed
Whiteboard: [qa-]
Attachment #597203 - Flags: approval-mozilla-esr10?
Attachment #597203 - Flags: approval-mozilla-esr10? → approval-mozilla-esr10+
https://hg.mozilla.org/releases/mozilla-esr10/rev/566be745f397
status-firefox-esr10: --- → fixed
You need to log in before you can comment on or make changes to this bug.