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.

Sign up for GitHub

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

Jump to bottom

Ask Brave for suggestion needs to be removed #1623

Closed
srirambv opened this issue Oct 16, 2018 · 4 comments · Fixed by brave/brave-core#682
Closed

Ask Brave for suggestion needs to be removed #1623

srirambv opened this issue Oct 16, 2018 · 4 comments · Fixed by brave/brave-core#682
Assignees
@mkarolin
Labels
about-pages/rebrand Rebranding work on top of Chrome's Polymer-powered pages feature/context-menu feature/user-interface All UI related OS/Linux OS/Windows QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Test-Plan-Specified QA/Yes release/blocking release-notes/exclude
Milestone
0.56.x - Release ...

Comments

@srirambv
Copy link
Contributor

srirambv commented Oct 16, 2018 •
edited by bsclifton
Loading

Test plan

See brave/brave-core#682

Description

Ask Brave for suggestion needs to be removed

Steps to Reproduce

  1. Visit any site and right click on a text field
  2. Click on SpellCheck->Ask Brave for suggestion
  3. Modal comes up which should be blocked as no data is being sent to any Brave server
    image

Actual result:

spellcheck

Expected result:

Should not show this. Issue was reported on #221 but was just renamed to Brave but not removed

Reproduces how often:

Easy

Brave version (brave://version info)

Brave 0.55.14 Chromium: 70.0.3538.54 (Official Build) beta (64-bit)
Revision 4f8e578b6680574714e9ed3bb9f02922b4dde40d-refs/branch-heads/3538@{#937}
OS All

Reproducible on current release:

  • Does it reproduce on brave-browser dev/beta builds?
    Yes
  • Does it reproduce on browser-laptop?
    No

Website problems only:

  • Does the issue resolve itself when disabling Brave Shields?
  • Is the issue reproducible on the latest version of Chrome?

Additional Information
@srirambv srirambv added feature/user-interface All UI related release/blocking feature/context-menu about-pages/rebrand Rebranding work on top of Chrome's Polymer-powered pages labels Oct 16, 2018
@srirambv srirambv added this to the 0.57.x - Dev milestone Oct 16, 2018
@srirambv srirambv mentioned this issue Oct 16, 2018
Closed
25 tasks
@srirambv srirambv modified the milestones: 0.57.x - Dev, 0.56.x - Beta Oct 16, 2018
@rebron rebron modified the milestones: 1.0, 0.56.x - Beta Oct 16, 2018
@mkarolin mkarolin self-assigned this Oct 18, 2018
mkarolin added a commit to brave/brave-core that referenced this issue Oct 18, 2018
@mkarolin
Hide RenderViewContextMenu::AddSpellCheckServiceItem method with
1d53a44 
one in BraveRenderViewContextMenu that does not add Ask Brave menu item.

Fixes brave/brave-browser#1623
@mkarolin mkarolin mentioned this issue Oct 18, 2018
Merged
18 tasks
@bsclifton
Copy link
Member

I can't seem to reproduce the problem on macOS... but I can on Windows. @srirambv @btlechowski does this also show on Linux?

@srirambv
Copy link
Contributor Author

@bsclifton yes I can see this on both Linux and Windows

mkarolin added a commit to brave/brave-core that referenced this issue Oct 24, 2018
@mkarolin
Hide RenderViewContextMenu::AddSpellCheckServiceItem method with
69d1689 
one in BraveRenderViewContextMenu that does not add Ask Brave menu item.

Fixes brave/brave-browser#1623

Also, overrides SpellingMenuObserver to remove a possible dangling menu
separator. Adds browser tests for spelling menu and spelling options
submenu that check that the "Ask Brave for suggestions" menu item is not
present.
bsclifton pushed a commit to brave/brave-core that referenced this issue Oct 29, 2018
@mkarolin @bsclifton
Hide RenderViewContextMenu::AddSpellCheckServiceItem method with
c82efbc 
one in BraveRenderViewContextMenu that does not add Ask Brave menu item.

Fixes brave/brave-browser#1623

Also, overrides SpellingMenuObserver to remove a possible dangling menu
separator. Adds browser tests for spelling menu and spelling options
submenu that check that the "Ask Brave for suggestions" menu item is not
present.
@rebron rebron removed this from the 0.56.x - Beta milestone Oct 30, 2018
@bsclifton bsclifton added this to the 0.56.x - Beta milestone Oct 30, 2018
@LaurenWags
Copy link
Member

LaurenWags commented Nov 1, 2018 •
edited by srirambv
Loading

Verified passed on

Brave 0.56.7 Chromium: 70.0.3538.77 (Official Build) beta(64-bit)
Revision 0f6ce0b0cd63a12cb4eccea3637b1bc9a29148d9-refs/branch-heads/3538@{#1039}
OS Mac OS X
  • Verified option is still not available on macOS

Verification Passed on

Brave 0.56.8 Chromium: 70.0.3538.77 (Official Build) (64-bit)
Revision 0f6ce0b0cd63a12cb4eccea3637b1bc9a29148d9-refs/branch-heads/3538@{#1039}
OS Windows

Went through verification using the following build under Ubuntu 18.04 x64 & Mint 18.3 x64 - PASSED

Brave 0.56.8 Chromium: 70.0.3538.77 (Official Build) (64-bit)
Revision 0f6ce0b0cd63a12cb4eccea3637b1bc9a29148d9-refs/branch-heads/3538@{#1039}
OS Linux
  • checked several text fields and ensured that Ask Brave for suggestion was removed under the SpellCheck context menu when using right click

Verification Passed on

Brave 0.56.8 Chromium: 70.0.3538.77 (Official Build) (64-bit)
Revision 0f6ce0b0cd63a12cb4eccea3637b1bc9a29148d9-refs/branch-heads/3538@{#1039}
OS Linux

image

@kjozwiak
Copy link
Member

kjozwiak commented Nov 5, 2018

@srirambv seems to be working correctly under Ubuntu 18.04 x64 & Mint 18.3 x64. Can you quickly double check to make sure it's working for you as you're the one that initially reported it? You can add the QA Pass-Linux if it's working for you as well 👍

@kjozwiak kjozwiak mentioned this issue Nov 9, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mkarolin mkarolin

Labels
about-pages/rebrand Rebranding work on top of Chrome's Polymer-powered pages feature/context-menu feature/user-interface All UI related OS/Linux OS/Windows QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Test-Plan-Specified QA/Yes release/blocking release-notes/exclude
Projects
None yet
Milestone
0.56.x - Release (Release Channel)
Development

Successfully merging a pull request may close this issue.

Remove "Ask Brave for suggestions" from context menu spellcheck section brave/brave-core
7 participants
@kjozwiak @bsclifton @rebron @srirambv @LaurenWags @GeetaSarvadnya @mkarolin