Closed Bug 1287622 Opened 8 years ago Closed 8 years ago

Remove Cortana-related code from mozilla-central

Categories

(Firefox :: Search, defect, P4)

defect

Tracking

()

RESOLVED FIXED
Firefox 52
Tracking Status
firefox50 --- affected
firefox52 --- fixed

People

(Reporter: jaws, Assigned: u579587, Mentored)

References

Details

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

Attachments

(1 file, 3 obsolete files)

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: fx-qx
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]
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: nobody → bugzilla
Status: NEW → ASSIGNED
Attachment #8791734 - Attachment is obsolete: true
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+
Flags: needinfo?(jaws)
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.
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.
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
(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!
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
https://hg.mozilla.org/mozilla-central/rev/fd5453b8f5b8
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
You need to log in before you can comment on or make changes to this bug.