Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide google microphone from https://github.com/brave/brave-browser/issues/2690 #3978

Merged
merged 2 commits into from
Jan 13, 2020

Conversation

ryanbr
Copy link
Collaborator

@ryanbr ryanbr commented Nov 12, 2019

Fixes brave/brave-browser#2690

Since the google microphone doesn't work, the microphone is hidden on Firefox (possibly a user agent checks by google)

This patch will hide the microphone icon on all google regional sites, tested on www.google.com, www.google.co.nz, www.google.com.au, www.google.co.uk, www.google.de This patch works both in desktop and mobile versions of google.

  • Main home page, microphone hidden
  • When google searching, microphone hidden

@bsclifton
Copy link
Member

@ryanbr can you rebase and squash the commits? 😄 And then update the commit message to include Fixes https://github.com/brave/brave-browser/issues/2690. Will give it a review after that 😄

Sorry that this seems to have gotten lost among the sea of other PRs

@bsclifton bsclifton added this to the 1.5.x - Nightly milestone Dec 30, 2019
@bbondy
Copy link
Member

bbondy commented Jan 14, 2020

FYI this was reverted here:
#4371

Because of test failures that this introduced:

[357/357] BraveWalletAPIBrowserTest.DappDetectionWithMetaMaskTestCancel (TIMED OUT)
3 tests timed out:
    BraveWalletAPIBrowserTest.DappDetectionTestAccept (../../brave/browser/extensions/api/brave_wallet_api_browsertest.cc:158)
    BraveWalletAPIBrowserTest.DappDetectionWithMetaMaskTestAccept (../../brave/browser/extensions/api/brave_wallet_api_browsertest.cc:175)
    BraveWalletAPIBrowserTest.DappDetectionWithMetaMaskTestCancel (../../brave/browser/extensions/api/brave_wallet_api_browsertest.cc:191)

I'll re-open the issue.

@bsclifton
Copy link
Member

@ryanbr as noted above, this caused some test failures. I believe the content script changes were causing the Brave extension to crash, which also hid the Shields (Lion) icon

Tests were missed because this was a fork. Would you be able to re-submit this PR as a branch? Let me know if you don't have privs for branching and I can create for you 😄 Then we'll just need to track down why it's causing a problem

@ryanbr
Copy link
Collaborator Author

ryanbr commented Jan 16, 2020

Yeah can create branch PR, is the orginal code sound @bsclifton ? We could simplify it more to something like this:

var selection = document.querySelector('[aria-label="Search by voice"]'); selection.style.display = 'none';

@DrMWEcker
Copy link

DrMWEcker commented Jan 20, 2020

Just sticking in my two cents.
Tried Brave on one of my many PCs last summer. I didn't make it the default or use it since.
Today (20 January 2020) I gave it a try again, but allowing Google as default search because of certain advantages. Unfortunately, voice search and dictation are a big part of the Chrome + Google advantage for me, and I was disappointed that voice search is still a problem for Brave in 2020.

Perhaps it will eventually get fixed and I will hear of this, as I would likely come back to Brave. Alas, until this is fixed, I will have to be brave without using Brave. Ciao!

@ryanbr
Copy link
Collaborator Author

ryanbr commented Mar 11, 2020

ping @bsclifton not sure you missed my comment ^, not sure how the Shields icon was affected, but would simplified check be better?

@bsclifton
Copy link
Member

@ryanbr really sorry for missing this (finally getting to older GitHub notifications, doh)

Simplified check would be better- I'm not sure what the root cause was before. In Nightly, the Brave extension was crashing so maybe it's just a matter of pulling down the code and trying it, in case you wanted to submit another PR. Feel free to ping me on Slack too! 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Google search-by-voice displays "no internet connection"
5 participants