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

Disable Make a Call From option #8454

Merged
merged 2 commits into from
Apr 12, 2021
Merged

Conversation

AlexeyBarabash
Copy link
Contributor

Resolves brave/brave-browser#14601, brave/brave-browser#11526

Share call Chromium feature uses sync to get devices list and uses Google Cloud Messaging to push the call to the Android device. Therefore it is not supported by Brave browser.

The proper way would be to use gn variable enable_click_to_call, but setting it to false gives compile errors:

../../chrome/browser/sharing/click_to_call/phone_number_regex.cc:43:37: error: use of undeclared identifier 'kClickToCallUI'
  if (!base::FeatureList::IsEnabled(kClickToCallUI))


../../chrome/browser/sharing/click_to_call/click_to_call_utils.cc:40:58: error: no member named 'kClickToCallEnabled' in namespace 'prefs'
  if (profile && !profile->GetPrefs()->GetBoolean(prefs::kClickToCallEnabled))
                                                  ~~~~~~~^
../../chrome/browser/sharing/click_to_call/click_to_call_utils.cc:45:58: error: use of undeclared identifier 'kClickToCallUI'
  return sharing_service && base::FeatureList::IsEnabled(kClickToCallUI);
                                                         ^

../../brave/chromium_src/chrome/browser/ui/views/location_bar/../../../../../../../chrome/browser/ui/views/location_bar/location_bar_view.cc:266:38: error: use of undeclared identifier 'kClickToCallUI'
    if (base::FeatureList::IsEnabled(kClickToCallUI))

By this reason override of ShouldOfferClickToCallForURL is used.

Submitter Checklist:

  • I confirm that no security/privacy review is needed, or that I have requested one
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint// tested lint only
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

  1. Put the desktop into the same sync chain with an Android device
  2. Open the website which contains tel: link, like https://www.w3schools.com/tags/tryit.asp?filename=tryhtml_link_phoneto
  3. Click the link with phone number
  4. Expected: there is no dialog which choose the device to call from like
    image

@AlexeyBarabash AlexeyBarabash requested a review from a team as a code owner April 6, 2021 21:15
@AlexeyBarabash AlexeyBarabash self-assigned this Apr 6, 2021
@AlexeyBarabash AlexeyBarabash requested a review from darkdh April 6, 2021 21:15
@AlexeyBarabash AlexeyBarabash force-pushed the disable_sync_phone_call branch from ec854d6 to 8a69445 Compare April 7, 2021 10:19
@AlexeyBarabash AlexeyBarabash requested a review from bridiver as a code owner April 7, 2021 12:11
@AlexeyBarabash
Copy link
Contributor Author

Linux CI browser-test failed


16:35:48  1 test crashed:
16:35:48      RewardsPublisherBrowserTest.VisitRegisteredPublisher (../../brave/components/brave_rewards/browser/test/rewards_publisher_browsertest.cc:157)

and

16:40:31  2 tests crashed:
16:40:31      RewardsContributionBrowserTest.AutoContributionMultiplePublishersUphold (../../brave/components/brave_rewards/browser/test/rewards_contribution_browsertest.cc:214)
16:40:31      RewardsPublisherBrowserTest.VisitUnverifiedPublisher (../../brave/components/brave_rewards/browser/test/rewards_publisher_browsertest.cc:147)

not related to the PR

macOS CI browser-test failed


02:31:58  1 test failed:
02:31:58      BraveNavigatorUserAgentFarblingMobileBrowserTest.FarbleNavigatorUserAgentMobile (../../brave/browser/farbling/brave_navigator_useragent_farbling_browsertest.cc:218)

not related to the PR

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

Successfully merging this pull request may close these issues.

Remove / Disable "Make a Call From" Chromium dialog
3 participants