Closed Bug 1243507 Opened 4 years ago Closed 4 years ago

Drag & drop from external application (keepass 1.30) no longer possible in Firefox 44

Categories

(Core :: Widget: Win32, defect)

44 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox44 --- wontfix
firefox45 --- fixed
firefox46 --- fixed
firefox47 --- fixed

People

(Reporter: philipp, Assigned: emk)

References

Details

(Keywords: regression)

Attachments

(1 file, 2 obsolete files)

we had 3 independent users reporting that they were no longer able to drag & drop text (username/password entries) from the external keepass 1.30 tool (later keepass 2 versions do work) into login forms since they updated to firefox 44. 

this is easily reproducible and regression testing this leads to https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=3df3d4856542e8ebad6262becab79058fb592b71&tochange=8ca7268d0d1f17375e870a43982b18a76a6fbd50 and bug 943294 as cause for this.

i don't know if this would be an intended by-product of bug 943294, but wanted to get it on file at least...
This is not expected. We expected that we would have no user-visible change.

My speculation is:
Although Windows Clipboard will automatically convert CF_TEXT for us, drag and drop will pass any random IDataObj that may not support CF_UNICODETEXT because the drop source app implements IDataObj, not the Windows system.

We will have to backout bug 943294 to support drag ang drop from Unicode-unaware apps :(
yes, the issue is fixed in both those try builds.
Flags: needinfo?(madperson)
This is a straight backout generated by "hg backout".
This patch will not compile because we removed nsPrimitiveHelpers::ConvertPlatformPlainTextToUnicode. I'll fix it in the next patch.
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
Attachment #8713133 - Flags: review?(jmathies)
See <https://hg.mozilla.org/mozilla-central/rev/01a5ca4ef014#l1.84> about the removed ConvertPlatformPlainTextToUnicode.
I'll fold two patches before landing.
Attachment #8713135 - Flags: review?(jmathies)
So very sad that Windows isn't consistent across clipboard and D&D--especially considering how Microsoft's clipboard docs endorse the approach taken in bug 943294.

Did you happen to try to research if there's some way to ask the D&D system to behave like the clipboard and do the encoding conversion on the system side?
I'd like to keep the change minimal because I consider to uplift once this fix landed on m-c.
The restored code should be harmless to clipboard operations because CF_UNICODETEXT should always be found.

Please file a follow-up bug for further improvements.
Blocks: 1245129
Will this patch fix the opposite operation, id est, drag and drop text from Firefox to external application (like Bing search field in IE)?
No, apparently it is a different problem. Notepad and Internet Explorer are Unicode apps.
Jim, ping?
Please redirect to another reviewer if you busy. I would like to fix this early in the cycle for uplift.
Flags: needinfo?(jmathies)
No longer blocks: 1245129
Attachment #8713133 - Flags: review?(jmathies) → review+
Comment on attachment 8713135 [details] [diff] [review]
Reimplement FindUnicodeFromPlainText using NS_CopyNativeToUnicode

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

sorry for the delay.

::: widget/windows/nsClipboard.cpp
@@ +791,2 @@
>  
> +  const char* castedText = static_cast<char*>(*outData);          

nit - ws

@@ +791,4 @@
>  
> +  const char* castedText = static_cast<char*>(*outData);          
> +  nsAutoString tmp;
> +  nsresult rv = NS_CopyNativeToUnicode(nsDependentCSubstring(castedText, *outDataLen), tmp);

lets change 'loadResult' to 'rv', and just reuse it.

@@ +795,5 @@
> +  if (NS_FAILED(rv)) {
> +    return false;
> +  }
> +
> +  // out with the old, in with the new 

nit - ws
Attachment #8713135 - Flags: review?(jmathies) → review+
Flags: needinfo?(jmathies)
Blocks: 1245129
https://hg.mozilla.org/integration/mozilla-inbound/rev/ebb0d8d11b23ed0373df527cf62a04a234896c8c
Bug 1243507 - Reimplement non-Unicode clipboard formats as bug 943294 broke drag and drop between some Unicode-unaware apps. r=jimm
Attachment #8713133 - Attachment is obsolete: true
Attachment #8713135 - Attachment is obsolete: true
Attachment #8715418 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/ebb0d8d11b23
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Duplicate of this bug: 1245488
Comment on attachment 8715418 [details] [diff] [review]
patch for checkin

Approval Request Comment
[Feature/regressing bug #]: bug 943294
[User impact if declined]: Users can not drag and drop text between Firefox and some apps.
[Describe test coverage new/current, TreeHerder]: manually tested
[Risks and why]: Low. Although the patch looks large, most of changes are a simple backout.
[String/UUID change made/needed]: none
Attachment #8715418 - Flags: approval-mozilla-beta?
Attachment #8715418 - Flags: approval-mozilla-aurora?
Comment on attachment 8715418 [details] [diff] [review]
patch for checkin

Simple backout, taking it.
Should be in 45 beta 3.
Attachment #8715418 - Flags: approval-mozilla-beta?
Attachment #8715418 - Flags: approval-mozilla-beta+
Attachment #8715418 - Flags: approval-mozilla-aurora?
Attachment #8715418 - Flags: approval-mozilla-aurora+
No longer blocks: 1245129
Duplicate of this bug: 1245129
(In reply to Masatoshi Kimura [:emk] from comment #9)
> I'd like to keep the change minimal because I consider to uplift once this
> fix landed on m-c.
> The restored code should be harmless to clipboard operations because
> CF_UNICODETEXT should always be found.
> 
> Please file a follow-up bug for further improvements.

Thanks for fixing this.

I suppose further improvements aren't worthwhile at this point: since we now do the legacy conversion using Windows' own converters, getting Windows to do it for us wouldn't be much of a win.

At least we still got rid of the nsPlatformCharset usage.
Just to add: This bug shows itself with Outlook 2016. You can drag and drop onto a subject field, but not the bulk of an email.
Duplicate of this bug: 1252824
Duplicate of this bug: 1254781
Is this the only clipboard format related change in Firefox 45? Two users on SuMo report that Thunderbird isn't finding HTML on the clipboard when pasting from Firefox 45. ??

https://support.mozilla.org/questions/1113486
https://support.mozilla.org/questions/1113511
Oops, never mind: new bug already filed this morning for the Firefox 45 copy / Thunderbird paste problem: https://bugzilla.mozilla.org/show_bug.cgi?id=1254980
You need to log in before you can comment on or make changes to this bug.