Skip to content

Commit ac58edb

Browse files
malmsteinmalmstein
authored
Fix custom tab mode: (#5829)
Task/Issue URL: https://app.asana.com/0/1157893581871903/1209822654738726/f ### Description The custom tab mode wasn’t rendering properly. This PR fixes it ### Steps to test this PR _Custom Tab_ - [x] Fresh install app and set it as default browser - [x] Go to an app that will allow you to open a link (Reddit) - [x] Tap on a link - [x] Verify Custom Tab is displayed properly (tabs button is not visible, X is visible) - [x] Open overflow menu and tap on “Open in DuckDuckGo" - [x] Verify that the omnibar is displayed with the tabs button and without the X _Malicious site protection_ - [x] Open https://privacy-test-pages.site/security/badware/ - [x] Navigate to https://privacy-test-pages.site/security/badware/phishing-iframe-loader.html - [x] Once the error page is shown, open a new tab and load [wikipedia.org](https://wikipedia.org/) - [x] While on the wikipedia tab, close and kill the app - [x] Reopen the app, and wait for wikipedia to load - [x] Switch to the previous tab - [x] Check globe icon is shown
1 parent 871ef18 commit ac58edb

File tree

6 files changed

+99
-56
lines changed

6 files changed

+99
-56
lines changed

‎.maestro/custom_tabs/custom_tabs_navigation.yaml‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,18 @@ tags:
4848
- inputText: "https://www.search-company.site"
4949
- tapOn:
5050
text: "load custom tab"
51+
- assertNotVisible:
52+
id: "com.duckduckgo.mobile.android:id/fireIconImageView"
53+
- assertNotVisible:
54+
id: "com.duckduckgo.mobile.android:id/tabsMenu"
55+
- assertVisible:
56+
id: "com.duckduckgo.mobile.android:id/customTabCloseIcon"
57+
- assertVisible:
58+
id: "com.duckduckgo.mobile.android:id/customTabShieldIcon"
59+
- assertVisible:
60+
id: "com.duckduckgo.mobile.android:id/customTabTitle"
61+
- assertVisible:
62+
id: "com.duckduckgo.mobile.android:id/customTabDomain"
5163
- tapOn:
5264
id: "com.duckduckgo.mobile.android:id/browserMenuImageView"
5365
- assertVisible:

‎app/src/main/java/com/duckduckgo/app/browser/menu/BrowserPopupMenu.kt‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import com.duckduckgo.app.browser.omnibar.model.OmnibarPosition.TOP
3030
import com.duckduckgo.app.browser.viewstate.BrowserViewState
3131
import com.duckduckgo.common.ui.menu.PopupMenu
3232
import com.duckduckgo.common.ui.view.MenuItemView
33-
import com.duckduckgo.mobile.android.R.dimen
3433
import com.duckduckgo.mobile.android.R.drawable
3534

3635
class BrowserPopupMenu(
@@ -276,7 +275,7 @@ class BrowserPopupMenu(
276275

277276
printPageMenuItem.isEnabled = browserShowing
278277

279-
newTabMenuItem.isVisible = browserShowing && !displayedInCustomTabScreen
278+
newTabMenuItem.isVisible = !displayedInCustomTabScreen
280279
duckChatMenuItem.isVisible = viewState.showDuckChatOption && !displayedInCustomTabScreen
281280
sharePageMenuItem.isVisible = viewState.canSharePage
282281

@@ -348,13 +347,14 @@ class BrowserPopupMenu(
348347
openInDdgBrowserMenuItem.isVisible = displayedInCustomTabScreen
349348
customTabsMenuDivider.isVisible = displayedInCustomTabScreen
350349
runningInDdgBrowserMenuItem.isVisible = displayedInCustomTabScreen
351-
overrideForSSlError(viewState)
350+
overrideForSSlError(viewState, displayedInCustomTabScreen)
352351
}
353352

354353
private fun overrideForSSlError(
355354
viewState: BrowserViewState,
355+
displayedInCustomTabScreen: Boolean,
356356
) {
357-
if (viewState.sslError != NONE) {
357+
if (viewState.sslError != NONE && !displayedInCustomTabScreen) {
358358
newTabMenuItem.isVisible = true
359359
siteOptionsMenuDivider.isVisible = true
360360
}

‎app/src/main/java/com/duckduckgo/app/browser/omnibar/Omnibar.kt‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ class Omnibar(
8585
binding.rootView.removeView(binding.fadeOmnibar)
8686
binding.rootView.removeView(binding.fadeOmnibarBottom)
8787
}
88+
8889
FADE -> {
8990
// remove bottom variant
9091
binding.rootView.removeView(binding.fadeOmnibarBottom)
@@ -106,6 +107,7 @@ class Omnibar(
106107
binding.rootView.removeView(binding.fadeOmnibar)
107108
binding.rootView.removeView(binding.fadeOmnibarBottom)
108109
}
110+
109111
FADE -> {
110112
// remove top variant
111113
binding.rootView.removeView(binding.fadeOmnibar)
@@ -174,6 +176,7 @@ class Omnibar(
174176
data class Browser(val url: String?) : ViewMode()
175177
data class CustomTab(
176178
val toolbarColor: Int,
179+
val title: String?,
177180
val domain: String?,
178181
val showDuckPlayerIcon: Boolean = false,
179182
) : ViewMode()
@@ -237,6 +240,7 @@ class Omnibar(
237240
}
238241

239242
fun setViewMode(viewMode: ViewMode) {
243+
Timber.d("Omnibar: setViewMode $viewMode")
240244
when (viewMode) {
241245
Error -> {
242246
newOmnibar.decorate(Mode(viewMode))
@@ -389,7 +393,7 @@ class Omnibar(
389393
customTabToolbarColor: Int,
390394
customTabDomainText: String?,
391395
) {
392-
newOmnibar.decorate(Mode(CustomTab(customTabToolbarColor, customTabDomainText)))
396+
newOmnibar.decorate(Mode(CustomTab(toolbarColor = customTabToolbarColor, title = null, domain = customTabDomainText)))
393397
}
394398

395399
fun showWebPageTitleInCustomTab(
@@ -427,6 +431,7 @@ class Omnibar(
427431
SCROLLING -> {
428432
// no-op
429433
}
434+
430435
FADE -> binding.fadeOmnibar.onScrollChanged(
431436
scrollableView = v,
432437
scrollY = scrollY,
@@ -440,6 +445,7 @@ class Omnibar(
440445
SCROLLING -> {
441446
// no-op
442447
}
448+
443449
FADE -> binding.fadeOmnibarBottom.onScrollChanged(
444450
scrollableView = v,
445451
scrollY = scrollY,

‎app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarLayout.kt‎

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -256,11 +256,13 @@ open class OmnibarLayout @JvmOverloads constructor(
256256
}
257257

258258
if (lastViewMode != null) {
259+
Timber.d("Omnibar: onAttachedToWindow lastViewMode $lastViewMode")
259260
decorateDeferred(lastViewMode!!)
260261
lastViewMode = null
261262
}
262263

263264
if (decoration != null) {
265+
Timber.d("Omnibar: onAttachedToWindow decoration $decoration")
264266
decorateDeferred(decoration!!)
265267
decoration = null
266268
}
@@ -534,17 +536,22 @@ open class OmnibarLayout @JvmOverloads constructor(
534536
) {
535537
Timber.d("Omnibar: renderCustomTabMode $viewState")
536538
configureCustomTabOmnibar(viewMode)
539+
renderCustomTab(viewMode)
537540
}
538541

539542
fun decorate(decoration: Decoration) {
543+
Timber.d("Omnibar: decorate $decoration")
540544
if (isAttachedToWindow) {
541545
decorateDeferred(decoration)
542546
} else {
543547
/* TODO (cbarreiro): This is a temporary solution to prevent one-time decorations causing mode to be lost when view is not attached
544548
* As a long-term solution, we should move mode to StateChange, and only have one-time decorations here
545549
*/
546550
if (decoration is Mode) {
547-
lastViewMode = decoration
551+
val lastMode = lastViewMode?.viewMode
552+
if (lastMode !is CustomTab) {
553+
lastViewMode = decoration
554+
}
548555
this.decoration = null
549556
} else if (this.decoration == null) {
550557
this.decoration = decoration
@@ -579,7 +586,7 @@ open class OmnibarLayout @JvmOverloads constructor(
579586
}
580587

581588
is ChangeCustomTabTitle -> {
582-
updateCustomTabTitle(decoration)
589+
viewModel.onCustomTabTitleUpdate(decoration)
583590
}
584591

585592
is HighlightOmnibarItem -> {
@@ -696,10 +703,6 @@ open class OmnibarLayout @JvmOverloads constructor(
696703

697704
browserMenu.isVisible = true
698705

699-
customTabToolbarContainer.customTabDomain.text = customTab.domain
700-
customTabToolbarContainer.customTabDomainOnly.text = customTab.domain
701-
customTabToolbarContainer.customTabDomainOnly.show()
702-
703706
val foregroundColor = calculateCustomTabBackgroundColor(customTab.toolbarColor)
704707
customTabToolbarContainer.customTabCloseIcon.setColorFilter(foregroundColor)
705708
customTabToolbarContainer.customTabDomain.setTextColor(foregroundColor)
@@ -709,19 +712,24 @@ open class OmnibarLayout @JvmOverloads constructor(
709712
}
710713
}
711714

712-
private fun updateCustomTabTitle(decoration: ChangeCustomTabTitle) {
715+
private fun renderCustomTab(viewMode: CustomTab) {
713716
Timber.d("Omnibar: updateCustomTabTitle $decoration")
714-
customTabToolbarContainer.customTabTitle.text = decoration.title
715717

716-
decoration.domain?.let {
717-
customTabToolbarContainer.customTabDomain.text = decoration.domain
718+
viewMode.domain?.let {
719+
customTabToolbarContainer.customTabDomain.text = viewMode.domain
720+
customTabToolbarContainer.customTabDomainOnly.text = viewMode.domain
721+
customTabToolbarContainer.customTabDomain.show()
722+
customTabToolbarContainer.customTabDomainOnly.show()
723+
}
724+
725+
viewMode.title?.let {
726+
customTabToolbarContainer.customTabTitle.text = viewMode.title
727+
customTabToolbarContainer.customTabTitle.show()
728+
customTabToolbarContainer.customTabDomainOnly.hide()
718729
}
719730

720-
customTabToolbarContainer.customTabTitle.show()
721-
customTabToolbarContainer.customTabDomainOnly.hide()
722-
customTabToolbarContainer.customTabDomain.show()
723-
customTabToolbarContainer.customTabShieldIcon.isInvisible = decoration.showDuckPlayerIcon
724-
customTabToolbarContainer.customTabDuckPlayerIcon.isVisible = decoration.showDuckPlayerIcon
731+
customTabToolbarContainer.customTabShieldIcon.isInvisible = viewMode.showDuckPlayerIcon
732+
customTabToolbarContainer.customTabDuckPlayerIcon.isVisible = viewMode.showDuckPlayerIcon
725733
}
726734

727735
private fun calculateCustomTabBackgroundColor(color: Int): Int {

‎app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarLayoutViewModel.kt‎

Lines changed: 52 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import com.duckduckgo.app.browser.omnibar.Omnibar.ViewMode.MaliciousSiteWarning
3131
import com.duckduckgo.app.browser.omnibar.Omnibar.ViewMode.NewTab
3232
import com.duckduckgo.app.browser.omnibar.Omnibar.ViewMode.SSLWarning
3333
import com.duckduckgo.app.browser.omnibar.OmnibarLayout.Decoration
34+
import com.duckduckgo.app.browser.omnibar.OmnibarLayout.Decoration.ChangeCustomTabTitle
3435
import com.duckduckgo.app.browser.omnibar.OmnibarLayout.Decoration.LaunchCookiesAnimation
3536
import com.duckduckgo.app.browser.omnibar.OmnibarLayout.Decoration.LaunchTrackersAnimation
3637
import com.duckduckgo.app.browser.omnibar.OmnibarLayout.StateChange
@@ -293,46 +294,51 @@ class OmnibarLayoutViewModel @Inject constructor(
293294
}
294295

295296
fun onViewModeChanged(viewMode: ViewMode) {
297+
val currentViewMode = _viewState.value.viewMode
296298
Timber.d("Omnibar: onViewModeChanged $viewMode")
297-
when (viewMode) {
298-
is CustomTab -> {
299-
_viewState.update {
300-
it.copy(
301-
viewMode = viewMode,
302-
showClearButton = false,
303-
showVoiceSearch = false,
304-
showBrowserMenu = true,
305-
showTabsMenu = false,
306-
showFireIcon = false,
307-
)
299+
if (currentViewMode is CustomTab) {
300+
Timber.d("Omnibar: custom tab mode enabled, sending updates there")
301+
} else {
302+
when (viewMode) {
303+
is CustomTab -> {
304+
_viewState.update {
305+
it.copy(
306+
viewMode = viewMode,
307+
showClearButton = false,
308+
showVoiceSearch = false,
309+
showBrowserMenu = true,
310+
showTabsMenu = false,
311+
showFireIcon = false,
312+
)
313+
}
308314
}
309-
}
310315

311-
else -> {
312-
val scrollingEnabled = viewMode != NewTab
313-
val hasFocus = _viewState.value.hasFocus
314-
val leadingIcon = if (hasFocus) {
315-
LeadingIconState.SEARCH
316-
} else {
317-
when (viewMode) {
318-
Error, SSLWarning, MaliciousSiteWarning -> GLOBE
319-
NewTab -> SEARCH
320-
else -> SEARCH
316+
else -> {
317+
val scrollingEnabled = viewMode != NewTab
318+
val hasFocus = _viewState.value.hasFocus
319+
val leadingIcon = if (hasFocus) {
320+
LeadingIconState.SEARCH
321+
} else {
322+
when (viewMode) {
323+
Error, SSLWarning, MaliciousSiteWarning -> GLOBE
324+
NewTab -> SEARCH
325+
else -> SEARCH
326+
}
321327
}
322-
}
323328

324-
_viewState.update {
325-
it.copy(
326-
viewMode = viewMode,
327-
leadingIconState = leadingIcon,
328-
scrollingEnabled = scrollingEnabled,
329-
showVoiceSearch = shouldShowVoiceSearch(
330-
hasFocus = _viewState.value.hasFocus,
331-
query = _viewState.value.omnibarText,
332-
hasQueryChanged = false,
333-
urlLoaded = _viewState.value.url,
334-
),
335-
)
329+
_viewState.update {
330+
it.copy(
331+
viewMode = viewMode,
332+
leadingIconState = leadingIcon,
333+
scrollingEnabled = scrollingEnabled,
334+
showVoiceSearch = shouldShowVoiceSearch(
335+
hasFocus = _viewState.value.hasFocus,
336+
query = _viewState.value.omnibarText,
337+
hasQueryChanged = false,
338+
urlLoaded = _viewState.value.url,
339+
),
340+
)
341+
}
336342
}
337343
}
338344
}
@@ -654,4 +660,15 @@ class OmnibarLayoutViewModel @Inject constructor(
654660
)
655661
}
656662
}
663+
664+
fun onCustomTabTitleUpdate(decoration: ChangeCustomTabTitle) {
665+
val customTabMode = viewState.value.viewMode
666+
if (customTabMode is CustomTab) {
667+
_viewState.update {
668+
it.copy(
669+
viewMode = customTabMode.copy(title = decoration.title),
670+
)
671+
}
672+
}
673+
}
657674
}

‎app/src/test/java/com/duckduckgo/app/browser/omnibar/OmnibarLayoutViewModelTest.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ class OmnibarLayoutViewModelTest {
287287

288288
@Test
289289
fun whenViewModeChangedToCustomTabThenViewStateCorrect() = runTest {
290-
testee.onViewModeChanged(ViewMode.CustomTab(0, "example.com", false))
290+
testee.onViewModeChanged(ViewMode.CustomTab(0, "example", "example.com", false))
291291

292292
testee.viewState.test {
293293
val viewState = awaitItem()

0 commit comments

Comments
 (0)