Last Comment Bug 724161 - simplify drag&drop checks for javascript:/data: URIs
: simplify drag&drop checks for javascript:/data: URIs
Status: RESOLVED FIXED
[qa-]
:
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All All
-- normal (vote)
: Firefox 13
Assigned To: :Gavin Sharp [email: gavin@gavinsharp.com]
:
:
Mentors:
Depends on: 718203
Blocks: CVE-2012-0455
  Show dependency treegraph
 
Reported: 2012-02-03 16:33 PST by :Gavin Sharp [email: gavin@gavinsharp.com]
Modified: 2012-02-29 11:49 PST (History)
2 users (show)
gavin.sharp: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed
fixed


Attachments
patch (2.51 KB, patch)
2012-02-03 16:36 PST, :Gavin Sharp [email: gavin@gavinsharp.com]
no flags Details | Diff | Splinter Review
patch (8.40 KB, patch)
2012-02-14 15:09 PST, :Gavin Sharp [email: gavin@gavinsharp.com]
dao+bmo: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
lukasblakk+bugs: approval‑mozilla‑esr10+
Details | Diff | Splinter Review

Description User image :Gavin Sharp [email: gavin@gavinsharp.com] 2012-02-03 16:33:55 PST
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 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2012-02-03 16:36:45 PST
Created attachment 594359 [details] [diff] [review]
patch
Comment 2 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2012-02-14 15:09:35 PST
Created attachment 597203 [details] [diff] [review]
patch

with tests
Comment 3 User image Dão Gottwald [:dao] 2012-02-15 04:17:26 PST
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 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2012-02-15 10:32:01 PST
(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 User image Dão Gottwald [:dao] 2012-02-16 05:56:02 PST
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 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2012-02-16 13:18:09 PST
(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 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2012-02-17 14:42:00 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/50aae34555bb
Comment 8 User image Ed Morley [:emorley] 2012-02-18 09:57:39 PST
https://hg.mozilla.org/mozilla-central/rev/50aae34555bb
Comment 9 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2012-02-18 18:27:18 PST
Comment on attachment 597203 [details] [diff] [review]
patch

[Approval Request Comment]
fix for bug 704354. depends on patch for bug 718203.
Comment 10 User image Alex Keybl [:akeybl] 2012-02-21 08:57:21 PST
Comment on attachment 597203 [details] [diff] [review]
patch

[Triage Comment]
Approving for Aurora/Beta in support of bug 704354. Please land today.
Comment 12 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2012-02-29 11:49:06 PST
https://hg.mozilla.org/releases/mozilla-esr10/rev/566be745f397

Note You need to log in before you can comment on or make changes to this bug.