Remove Cortana-related code from mozilla-central

RESOLVED FIXED in Firefox 52

Status

()

P4
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jaws, Assigned: u579587, Mentored)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 affected, firefox52 fixed)

Details

(Whiteboard: [good first bug][lang=js])

Attachments

(1 attachment, 3 obsolete attachments)

Due to bug 1286832, Cortana search no longer works in Firefox. We should remove the code from the tree since it's dead.
(To expand a bit, lest other bugs start getting duped to this one...)

Microsoft made a change to Windows 10 so that Cortana is now hard-coded to open search results only in Edge, and only with Bing. Previously, they respected your default browser choice, so that Firefox (or whatever your default was) would be launched, which could then use your default search engine. So this isn't a bug in Firefox, it's that Windows 10 will no longer send requests to Firefox in the first place.
Blocks: 1177237
I believe Microsoft rolled out this change with the recent Anniversary Update of Windows 10.
Blocks: 1244854
Priority: -- → P4
can we make this a mentored bug? Code removal sounds like something a contributor can easily handle.
Flags: needinfo?(jaws)
Good call, yes we can. I'll mentor it.

To fix this bug, we'll want to undo the changes from https://hg.mozilla.org/mozilla-central/rev/5d12635ce6be, https://hg.mozilla.org/mozilla-central/rev/c43d530f61c2, and https://hg.mozilla.org/mozilla-central/rev/7bc32b1953a1
Mentor: jaws
Flags: needinfo?(jaws)
Whiteboard: [good first bug][lang=js]
(Assignee)

Comment 6

2 years ago
Hi!

I'm at a Mozilla event looking for a first bug. I've a working build, yey :)

Can I work on this?

Thanks a bunch!
Yes, you may work on this. I will wait until you attach a patch before assigning the bug.
(Assignee)

Comment 8

2 years ago
Created attachment 8791734 [details] [diff] [review]
Remove Cortana-related code from Firefox Search, as it no longer works in Firefox after Microsoft hard-coded search results to Edge
Assignee: nobody → bugzilla
Status: NEW → ASSIGNED
(Assignee)

Comment 9

2 years ago
Created attachment 8791737 [details] [diff] [review]
Remove Cortana-related code from Firefox Search, as it no longer works in Firefox after Microsoft hard-coded search results to Edge
(Assignee)

Updated

2 years ago
Attachment #8791734 - Attachment is obsolete: true
(Assignee)

Comment 10

2 years ago
Thanks Jared! I'm winging this a lot. Can you check the patch?
Flags: needinfo?(jaws)
Comment on attachment 8791737 [details] [diff] [review]
Remove Cortana-related code from Firefox Search, as it no longer works in Firefox after Microsoft hard-coded search results to Edge

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

Looks good! Can you update the commit message to the following?
"Bug 1287622 - Remove Cortana-related code from Firefox as it no longer works after Microsoft hard-coded search results to Edge."

::: browser/app/profile/firefox.js
@@ -1517,4 @@
>  pref("browser.crashReports.unsubmittedCheck.enabled", true);
>  #endif
>  
> -pref("browser.crashReports.unsubmittedCheck.autoSubmit", false);
\ No newline at end of file

It looks like your patch added an extra newline at the end of this file.
Attachment #8791737 - Flags: review+
(Assignee)

Comment 12

2 years ago
Hey Jared! I'm a tad confused. We should have newlines at EOFs? Or did I add yet another? Because I can only the see the normal 1 newline at EOF. Sorry for being a newbie.

Also, I'm used to git, so basically I'm stuck trying to squash 2 commits and exporting a new patch.
(Assignee)

Comment 13

2 years ago
Created attachment 8791961 [details] [diff] [review]
0001-Bug-8791737-Remove-Cortana-related-code-from-Firefox.patch
Attachment #8791961 - Flags: review?(jaws)
Comment on attachment 8791961 [details] [diff] [review]
0001-Bug-8791737-Remove-Cortana-related-code-from-Firefox.patch

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

Yeah, only one newline at EOF. firefox.js already had a newline at the end of the file.

Squashing commits will work until you get a better hang of how to use Mercurial. I'll fix up the previous version of the patch and get it landed for you. I'll also see if I can find another bug for you to work on.

::: browser/app/profile/firefox.js
@@ -18,5 @@
> -#ifdef XP_UNIX
> -#ifndef XP_MACOSX
> -#define UNIX_BUT_NOT_MAC
> -#endif
> -#endif

These lines shouldn't be removed. The previous patch removed the "browser.search.redirectWindowsSearch" preference, which was correct.
Attachment #8791961 - Flags: review?(jaws) → review-
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #14)
> Comment on attachment 8791961 [details] [diff] [review]
> 0001-Bug-8791737-Remove-Cortana-related-code-from-Firefox.patch
> 
> Review of attachment 8791961 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Yeah, only one newline at EOF. firefox.js already had a newline at the end
> of the file.

I was wrong here, it looks like it didn't have a newline at the end of the file. I'm sorry for misleading you, your original change was fine.
Created attachment 8792507 [details] [diff] [review]
Patch for check-in
Attachment #8791737 - Attachment is obsolete: true
Attachment #8791961 - Attachment is obsolete: true
Attachment #8792507 - Flags: review+
I'm marking this patch as checkin-needed, which means that it should get checked in to mozilla-inbound within the next day or two. After it is checked in, it will get merged to mozilla-central a day or two later. Once it reaches mozilla-central, it will appear in Firefox Nightly the next day.

Congrats on getting your first bug fixed!
Keywords: checkin-needed
(Assignee)

Comment 18

2 years ago
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #17)
> I'm marking this patch as checkin-needed, which means that it should get
> checked in to mozilla-inbound within the next day or two. After it is
> checked in, it will get merged to mozilla-central a day or two later. Once
> it reaches mozilla-central, it will appear in Firefox Nightly the next day.
> 
> Congrats on getting your first bug fixed!

Thanks a million Jared! I'm now working with gecko-dev from GitHub + exporting the patch with git. It's a more comfortable setup for now. Thanks for your mentoring!

Comment 19

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/fx-team/rev/fd5453b8f5b8
Remove Cortana-related code from Firefox as it no longer works after Microsoft hard-coded search results to Edge. r=jaws
Keywords: checkin-needed

Comment 20

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fd5453b8f5b8
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Depends on: 1305794
You need to log in before you can comment on or make changes to this bug.