-
Notifications
You must be signed in to change notification settings - Fork 920
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
Fix a bug where pages can disable translation via html tags and the o… #27836
Conversation
@@ -325,6 +325,12 @@ class BraveTranslateTabHelper: NSObject { | |||
state: isTranslationSupported ? .available : .unavailable | |||
) | |||
|
|||
// Translation is not supported on this page, so do not show onboarding | |||
// Return immediately | |||
if !isTranslationSupported { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sure we don't translate from en
to en
for example. So a page cannot be translated if it's already in your desired language.
@@ -86,7 +86,6 @@ extension BrowserViewController: BraveTranslateScriptHandlerDelegate { | |||
action: { popover in | |||
popover.previewForOrigin = nil | |||
popover.dismissPopover() | |||
completion(nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixes the CheckedContinuation
crash. dismissPopover
already calls the completion
setTimeout(function() { | ||
try { | ||
try { | ||
if ((cr.googleTranslate.libReady || cr.googleTranslate.finished) && !cr.googleTranslate.readyCallback) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixes the Onboarding
button sometimes showing as if it's blended with reader-mode or in the wrong spot. It's because the page isn't ready to be translated yet, so onboarding showed up early.
The timeout didn't help, so it's removed in favour of just checking if the script is ready.
…nboarding showed up. Fixed a bug where translate popup showed up when the script wasn't ready. Fixed a crash in PopoverController usage calling CheckedContinuation twice.
7af8d22
to
c6d35b6
Compare
Released in v1.78.4 |
* Fix a bug where pages can disable translation via html tags and the onboarding showed up. * Fixed a bug where translate popup showed up when the script wasn't ready. * Fixed a crash in PopoverController usage calling CheckedContinuation twice.
* Fix a bug where pages can disable translation via html tags and the onboarding showed up. * Fixed a bug where translate popup showed up when the script wasn't ready. * Fixed a crash in PopoverController usage calling CheckedContinuation twice.
* Fix a bug where pages can disable translation via html tags and the onboarding showed up. * Fixed a bug where translate popup showed up when the script wasn't ready. * Fixed a crash in PopoverController usage calling CheckedContinuation twice.
* Fix a bug where pages can disable translation via html tags and the onboarding showed up. * Fixed a bug where translate popup showed up when the script wasn't ready. * Fixed a crash in PopoverController usage calling CheckedContinuation twice.
Summary
Resolves brave/brave-browser#44272
Submitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
wikinpm run presubmit
wiki,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan: