1361116 - Services menu in os x does not send pasteboard data to the service

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, defect, P1)

Description

[fiveNinePlusR]

Using services such as highlighting a word on a webpage and selecting the menu `Firefox | Services | Look up in Dictionary` fails to send the selected data to the pasteboard for processing by the service. 

mstange found the bug that caused this regression here: https://bugzilla.mozilla.org/show_bug.cgi?id=1235162

Comment 1

[Markus Stange [:mstange]]

[Tracking Requested - why for this release]: regression from Firefox 52

When this bug is fixed, we should re-check the STR in bug 1328994 comment 5 again, and possible resolve that bug.

Comment 2

[Marcia Knous [:marcia]]

Tracking 54/55+.

Comment 4

[Stephen A Pohl [:spohl]]

Attachment: Patch

macOS services are requesting the deprecated |NSStringPboardType| pasteboard type, so we need to accept it as a valid string pasteboard type or we will fail to write the selection to the pasteboard.

This patch only fixes the regression reported here and makes no claim to fix what was reported in bug 1328994 comment 5. Nevertheless, I will retest it later today or tomorrow.

Comment 5

[Markus Stange [:mstange]]

Comment on attachment 8868717 [details] [diff] [review]
Patch

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

::: widget/cocoa/nsChildView.mm
@@ +6234,1 @@
>        [types containsObject:htmlType] == NO) {

Let's convert all these three lines to use ![...] instead of [...] == NO.

Comment 6

[Stephen A Pohl [:spohl]]

Attachment: Patch

Carrying over r+. Updated this for the new patch 2 in bug 1358075.

(In reply to Markus Stange [:mstange] from comment #5)
> Comment on attachment 8868717 [details] [diff] [review]
> Patch
> 
> Review of attachment 8868717 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: widget/cocoa/nsChildView.mm
> @@ +6234,1 @@
> >        [types containsObject:htmlType] == NO) {
> 
> Let's convert all these three lines to use ![...] instead of [...] == NO.

Addressed this in patch 2 in bug 1358075.

Comment 7

[Stephen A Pohl [:spohl]]

Attachment: Patch

Updated for the new patch 2 in bug 1358075. Carrying over r+.

Comment 8

[Stephen A Pohl [:spohl]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/df442f1c10332426528aab20ea59d29682183939
Bug 1361116: Ensure that string data is sent to the pasteboard when requested by macOS services. r=mstange

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/df442f1c1033

Comment 10

[Nathan Froyd [:froydnj]]

Please request uplift on this when you get a chance.  Thanks!

Comment 11

[Stephen A Pohl [:spohl]]

Comment on attachment 8870270 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/Bug causing the regression]: bug 1235162
[User impact if declined]: Users will be unable to send strings to OSX/macOS services like they used to.
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: Yes, preferably. Steps in comment 0.
[List of other uplifts needed for the feature/fix]: bug 1358755, bug 1358075 and bug 1330470
[Is the change risky?]: no
[Why is the change risky/not risky?]: This is a tiny (3 loc) and well understood patch. 
[String changes made/needed]: none

Comment 12

[Julien Cristau [:jcristau]]

Comment on attachment 8870270 [details] [diff] [review]
Patch

aurora is about to die, redirecting to beta

Comment 13

[Gerry Chang [:gchang]]

Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!

Comment 14

[Brindusa Tot, Desktop QA]

Reproduced the bug with Nightly 55.0a1, from 05/10/2017 on Mac OS X 10.12.

Verified as fixed on the latest Nightly 55.0a1, Build ID 20170525030225 on Mac OS X 10.12.

Comment 15

[Julien Cristau [:jcristau]]

Comment on attachment 8870270 [details] [diff] [review]
Patch

fix regression from bug 1235162, beta54+

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/1db103c1ce1d

Comment 17

[Emil Ghitta, QA [:emilghitta]]

I have managed to reproduce the issue mentioned in comment 0 using Firefox 53.0.3 (Build Id:20170518000419).

This issue is verified fixed on Firefox 54.0 (Build Id:20170605134926) using macOS 10.12.1 and macOS 10.11.6.