1201623 - On tablets URL bar, about:home magnifying glass and globe icon take up different widths

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Theme and Visual Design, defect)

Description

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

STR:
 - Firefox on Tablet
 - Open a new tab
 - Click the URL bar
 - Repeat
 - Notice the icon shift the input cursor to the right when it changes from the globe icon to the search icon

Let's clean this up and make the padding the same.

Comment 1

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Perhaps mentor-able?

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

bug 1173652 was open to make these the same layouts, thus fixing this issue, but that would add a lot of code complexity and is maybe not worth it.

It's unclear how difficult this might be.

Comment 3

[Sergej Kravcenko]

Attachment: padding.diff

The padding was too big.
 
Display and edit layout merge will require way more work and must be done in corresponding bug.

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8663284 [details] [diff] [review]
padding.diff

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

This is better, but on my N9, the editing mode text still shifts to the right when the url bar is tapped.

Did you try rewriting the layout to use a more absolute positioning? e.g. the "favicon" container is always the same size, or the alignment happens from the right, or...

Comment 5

[Sergej Kravcenko]

Icon in edit layout is inside EditText as left drawable. To change this, icon must be moved to linear layout from edit text and corresponding code from ToolbarEditText to ToolbarEditLayout

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Sergej Kravcenko from comment #5)
> Icon in edit layout is inside EditText as left drawable. To change this,
> icon must be moved to linear layout from edit text and corresponding code
> from ToolbarEditText to ToolbarEditLayout

Works for me.

Comment 7

[Sergej Kravcenko]

Attachment: toolbar.diff

Should also fix bug 1173652.

This is not full merge, I've just made edit layout icon exact copy of display layout icon. The only thing that left is some misleading browser_toolbar_site_security_padding_* names inside toolbar_edit_layout.xml
Not sure how to generalise these names to look right.

Comment 8

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

I'll get to this tomorrow – don't forget to obsolete your old patch, like in the other bug!

Comment 9

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8664584 [details] [diff] [review]
toolbar.diff

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

Nice!

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=795cdd4674ed

Fix the comment, upload your patch, and feel free to add "checkin-needed" without re-review, provided the push is green.

Thanks Sergej!

::: mobile/android/base/resources/layout/toolbar_edit_layout.xml
@@ +6,5 @@
>  <merge xmlns:android="http://schemas.android.com/apk/res/android"
>         xmlns:gecko="http://schemas.android.com/apk/res-auto">
>  
> +    <!-- Search icon must have exact position and size as site security in
> +         display layout -->

Add an equivalent comment to the display layout.

Comment 10

[Sergej Kravcenko]

Attachment: search icon position fix

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/3aef96cabf2c

Comment 12

[Carsten Book [:Tomcat]]

Hi, had to back this out in https://hg.mozilla.org/integration/fx-team/rev/39a4d02f17db seems one of this changes caused a bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=4833952&repo=fx-team

Comment 13

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Looks like the failures were caused by bug 1144707 – the try in comment 9 is clean.

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/24644fbe805f

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/24644fbe805f