Closed Bug 1066250 Opened 5 years ago Closed 5 years ago

Consider always showing URL in toolbar in new tablet UI

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 35

People

(Reporter: lucasr, Assigned: mcomella)

References

Details

Attachments

(3 files, 2 obsolete files)

As we we'll show title in the tab strip anyway. No need to show title twice in the UI.
Assignee: nobody → michael.l.comella
Status: NEW → ASSIGNED
And it's like more desktop too!
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.
Attachment #8488282 - Flags: review?(lucasr.at.mozilla) → review+
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.
Attachment #8488285 - Flags: review?(lucasr.at.mozilla) → review+
Attachment #8488282 - Attachment is obsolete: true
Attachment #8488285 - Attachment is obsolete: true
Comment on attachment 8488947 [details] [diff] [review]
Part 3: Update testSettingsMenuItems

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

Looks good.
Attachment #8488947 - Flags: review?(lucasr.at.mozilla) → review+
You need to log in before you can comment on or make changes to this bug.