Lucas Rocha (:lucasr) Reporter
	Description • 10 years ago

As we we'll show title in the tab strip anyway. No need to show title twice in the UI.

	Yuan Wang(:Yuan) – Mobile Firefox Design Lead
	Comment 1 • 10 years ago

Sounds good to me!

	Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla] Assignee
	Comment 2 • 10 years ago

And it's like more desktop too!

	Lucas Rocha (:lucasr) Reporter
	Comment 5 • 10 years ago

Comment on attachment 8488282 [details] [diff] [review]
Part 1: Always show the url in the toolbar on new tablet

Review of attachment 8488282 [details] [diff] [review]:
-----------------------------------------------------------------

Nice.

::: mobile/android/base/toolbar/ToolbarDisplayLayout.java
@@ +322,5 @@
>              return;
>          }
>  
>          // If the pref to show the URL isn't set, just use the tab's display title.
> +        if (!mPrefs.shouldShowUrl(getContext()) || url == null) {

I think you can just use mActivity instead of getContext() here.

	Lucas Rocha (:lucasr) Reporter
	Comment 6 • 10 years ago

Comment on attachment 8488285 [details] [diff] [review]
Part 2: Remove title in toolbar preference on new tablet

Review of attachment 8488285 [details] [diff] [review]:
-----------------------------------------------------------------

You probably need to update testSettingsMenuItems accordingly.

::: mobile/android/base/preferences/GeckoPreferences.java
@@ +726,5 @@
>                              return true;
>                          }
>                      });
> +                } else if (PREFS_DISPLAY_TITLEBAR_MODE.equals(key) &&
> +                           NewTabletUI.isEnabled(getApplicationContext())) {

It's safe to just use 'this' instead of getApplicationContext() here.

	Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla] Assignee
	Comment 7 • 10 years ago

Address nit.

	Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla] Assignee
	Comment 8 • 10 years ago

Address nit.

	Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla] Assignee
	Comment 9 • 10 years ago

Tests in part 3.

	Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla] Assignee
	Comment 11 • 10 years ago

try for part 3 (comment 10): https://tbpl.mozilla.org/?tree=Try&rev=90ec2f68e989

	Lucas Rocha (:lucasr) Reporter
	Comment 12 • 10 years ago

Comment on attachment 8488947 [details] [diff] [review]
Part 3: Update testSettingsMenuItems

Review of attachment 8488947 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.

	Michael Comella (:mcomella) [NI reported issues only; no longer employed by Mozilla] Assignee
	Comment 13 • 10 years ago

https://hg.mozilla.org/integration/fx-team/rev/84d550c7cee2
https://hg.mozilla.org/integration/fx-team/rev/74e510d20ad3
https://hg.mozilla.org/integration/fx-team/rev/f00d45e4c159

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 14 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/84d550c7cee2
https://hg.mozilla.org/mozilla-central/rev/74e510d20ad3
https://hg.mozilla.org/mozilla-central/rev/f00d45e4c159