1246330 - Incorrect domain highlight

Status: RESOLVED WORKSFORME

(Firefox :: Address Bar, defect)

Description

[Toni Huttunen]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:43.0) Gecko/20100101 Firefox/43.0
Build ID: 20160105164030

Steps to reproduce:

1. Enter following address to the address bar: https://www.google.fi#asd
2. Do not enter to the site. Change cursor to the another place eg. click search bar


Actual results:

The string 'le.fi#asd' is boldfaced.


Expected results:

The string google.fi should be boldfaced.

Comment 1

[Toni Huttunen]

Here is a path to the boldface regexp:
/mozilla-central/blob/master/browser/base/content/urlbarBindings.xml

.. which is matchedURL = value.match(/^((?:[a-z]+:\/\/)?(?:[^\/]+@)?)(.+?)(?::\d+)?(?:\/|$)/);

Comment 2

[YF (Yang)]

https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=30d61283668592adfaec7a00aeeabc9d32a688c2&tochange=417e00a78107e0e97f35a3e4df4819cd91f4781d

https://hg.mozilla.org/integration/fx-team/rev/417e00a78107#l3.1


[Tracking Requested - why for this release]:
a regression since Fx43.

Comment 3

[Marco Bonardo [:mak]]

Gijs, do you know if we're expected to support that sytnax? looks like the fixup corrects it properly once confirmed

Comment 4

[:Gijs (he/him)]

(In reply to Marco Bonardo [::mak] from comment #3)
> Gijs, do you know if we're expected to support that sytnax? looks like the
> fixup corrects it properly once confirmed

It's not hard to improve that regex, no, so I believe we should do that.

Comment 5

[Marco Bonardo [:mak]]

Toni, would you mind making your suggestion in form of an actual patch we can review?

Comment 6

[Toni Huttunen]

Attachment: firefox-patch.txt

Comment 7

[Marco Bonardo [:mak]]

Comment on attachment 8716628 [details] [diff] [review]
firefox-patch.txt

let's keep this on the radar. Gijs if you come to this review before me, feel free to steal it.

Comment 8

[Marco Bonardo [:mak]]

Comment on attachment 8716628 [details] [diff] [review]
firefox-patch.txt

putting this on hold while we handle the dependency bug.

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Marco, are you planning to request an uplift to beta soon? Thanks

Comment 10

[:Gijs (he/him)]

(In reply to Sylvestre Ledru [:sylvestre] from comment #9)
> Marco, are you planning to request an uplift to beta soon? Thanks

This hasn't landed on Nightly yet. Please also see bug 1199601.

Comment 11

[:Gijs (he/him)]

I think this is now fixed by bug 1199601. Toni, can you verify this on current nightly or beta (7 or later)? Thank you!

Comment 12

[Toni Huttunen]

It works! @ beta45.0b8

Comment 13

[:Gijs (he/him)]

(In reply to Toni Huttunen from comment #12)
> It works! @ beta45.0b8

Great, thanks!