Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Invalid sync code alert needs proper heading #7280

Closed
srirambv opened this issue Feb 16, 2017 · 5 comments
Closed

Invalid sync code alert needs proper heading #7280

srirambv opened this issue Feb 16, 2017 · 5 comments

Comments

@srirambv
Copy link
Collaborator

srirambv commented Feb 16, 2017

Test plan

  1. Enable sync on the device and click on I have an existing sync code
  2. Enter wrong text and click Setup Sync
  3. Alert window should now look correct (showing "Brave" instead of extension ID)

Original issue description

  • Did you search for similar issues before submitting this one?

  • Describe the issue you encountered:
    Invalid sync code alert needs proper heading

  • Platform (Win7, 8, 10? macOS? Linux distro?):
    Windows 10 x64

  • Brave Version (revision SHA):
    Brave 0.13.5
    rev 59a41eb

  • Steps to reproduce:

    1. Enable sync on the device and click on I have an existing sync code
    2. Enter wrong text and click Setup Sync
    3. Alert window doesn't show proper heading
  • Actual result:
    Alert window doesn;t show proper heading

  • Expected result:
    Should show Brave Sync as the alert box heading

  • Will the steps above reproduce in a fresh profile? If not what other info can be added?

  • Is this an issue in the currently released version?

  • Can this issue be consistently reproduced?

  • Extra QA steps:
    1.
    2.
    3.

  • Screenshot if needed:
    image

  • Any related issues:
    cc: @ayumi

@srirambv srirambv added this to the 0.13.5 milestone Feb 16, 2017
@diracdeltas
Copy link
Member

@srirambv are you running Brave with @bsclifton's alert dialog changes? this is what i see on master:

screen shot 2017-02-16 at 10 17 16 pm

@bsclifton: maybe we can change the alert dialog to show friendly names for chrome-extension:// origins (like the name of the extension) instead of just the extension ID

@diracdeltas
Copy link
Member

diracdeltas commented Feb 16, 2017

there is actually some code already in brave/browser/brave_javascript_dialog_manager.cc that is supposed to handle extension alerts but it is commented out. for now this can be fixed with a simple string replacement: https://github.com/brave/browser-laptop/pull/7107/files#r101648919

@srirambv
Copy link
Collaborator Author

@diracdeltas I am using 0.13.5 P1 build not master

@bsclifton
Copy link
Member

@diracdeltas my code is still up in a PR- per your comment in the PR, I should be able to fix this up 😄 Thanks for assigning it over and providing more details

@srirambv
Copy link
Collaborator Author

srirambv commented Mar 1, 2017

Still same alert box on Preview 3. Verified on master (screenshot)
syncerror

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.