1644689 - Internal page location bar pages need sizing limitations

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Magnus Melin [:mkmelin]]

Trying to buy a mail account from within Thunderbird is kind of broken atm. The URL bar gets too wide, and the content is pushed out of view with no easy way to get to it.

Comment 1

[Magnus Melin [:mkmelin]]

The URL i'm talking about is for the page that opens once you select an option you want to buy.

Comment 2

[Magnus Melin [:mkmelin]]

Attachment: bug1644689_tb_locationbar_width.patch

Comment 3

[Richard Marti (:Paenglab)]

Comment on attachment 9155577 [details] [diff] [review]
bug1644689_tb_locationbar_width.patch

The label was chosen to not look like a URL bar and only show the address to not let the user think he could enter a different address. With your change it looks like one. A problem is also that not the whole URL is visible when it overflows and no tooltip is shown with the whole address.

How about only adding `word-break: break-word;` to the contentTabUrlbar class? This can end in multiple lines but the complete address is visible.

Comment 4

[Richard Marti (:Paenglab)]

And your solution misses more CSS to make the field look correct on dark themes.

Comment 5

[Magnus Melin [:mkmelin]]

But we're showing controls and all, so it is kind of a url bar. Only with readonly URL. If someone tries to change it they notice instantly they can't, so I don't see a problem with that.

With the location now in an input field it can be easily copied somewhere - which I've often needed for debug purposes at least. Looking at a tooltip wouldn't have helped me...

I don't want the address to wrap. When it does that it's very weird, and all it does is make things look ugly. Also browsers don't do that, for a good reason.

Any pointers to what's missing to make it work for dark theme?

Comment 6

[Richard Marti (:Paenglab)]

(In reply to Magnus Melin [:mkmelin] from comment #5)

Any pointers to what's missing to make it work for dark theme?

Adding the class themeableSearchBox should be enough.

Comment 7

[Magnus Melin [:mkmelin]]

Attachment: bug1644689_tb_locationbar_width.patch

Added theming

Comment 8

[Richard Marti (:Paenglab)]

Now, the site security icon looks not connected to the address. Can you try to move it inside the box?

Comment 9

[Richard Marti (:Paenglab)]

Attachment: 1644689-move-security-icon.patch

With this I'm okay with your patch.

Comment 10

[Richard Marti (:Paenglab)]

Comment on attachment 9155755 [details] [diff] [review]
bug1644689_tb_locationbar_width.patch

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

I give a r+ but the security icon should move into the inputbox.

::: mail/base/content/messenger.xhtml
@@ +811,5 @@
>                             disabled="true"/>
>              <toolbaritem class="contentTabAddress" align="center" flex="1">
>                <hbox align="center" flex="1">
>                  <image class="contentTabSecurity"/>
> +                <hbox flex="1" class="contentTabUrlbar input-container">

Flex is normally the last attribute. Could you move it to the end?

Comment 11

[Magnus Melin [:mkmelin]]

Comment on attachment 9156180 [details] [diff] [review]
1644689-move-security-icon.patch

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

Perfect, thx, I'll fold this in.

Comment 12

[Magnus Melin [:mkmelin]]

Attachment: bug1644689_tb_locationbar_width.patch

[Approval Request Comment]
Regression caused by (bug #): unclear
User impact if declined: Can't set up gandhi account, since their URL is long enough to hide the content off screen
Risk to taking this patch (and alternatives if risky): not risky

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/7ffd9252e1cf
limit the size of the URL location, and use an <html:input> instead, so that Copy/Paste context menu works for it. r=Paenglab DONTBUILD

Comment 14

[Magnus Melin [:mkmelin]]

Seems even 68 is affected by this! I wonder if the API changed to have a longer URL (server side), or how people are at all able to sign up atm... (it's possible under certain cases, but not at all all the time.

Comment 15

[Magnus Melin [:mkmelin]]

Comment on attachment 9156552 [details] [diff] [review]
bug1644689_tb_locationbar_width.patch

Needed for account creation to "work".

Comment 16

[Wayne Mery (:wsmwk)]

Comment on attachment 9156552 [details] [diff] [review]
bug1644689_tb_locationbar_width.patch

Approved for beta

Magnus, Not sure I see what you are seeing, so please retest beta after it ships as confirmation.

Comment 17

[Geoff Lankow (:darktrojan)]

Thunderbird 78.0b2:
https://hg.mozilla.org/releases/comm-beta/rev/49e5d2a19537

Comment 18

[Magnus Melin [:mkmelin]]

Confirmed, this part is working as it should in beta2.

Comment 19

[Wayne Mery (:wsmwk)]

Comment on attachment 9156552 [details] [diff] [review]
bug1644689_tb_locationbar_width.patch

Thanks for confirming beta.
Approved for esr68

Comment 20

[Rob Lemley [:rjl]]

Thunderbird 68.10.0:
https://hg.mozilla.org/releases/comm-esr68/rev/0579ee0b9174