Closed
Bug 1290490
Opened 8 years ago
Closed 8 years ago
Firefox shouldn't RTL the first item in the URL dropdown list when typing a URL
Categories
(Firefox :: Address Bar, defect, P2)
Tracking
()
RESOLVED
FIXED
Firefox 52
Tracking | Status | |
---|---|---|
firefox52 | --- | fixed |
People
(Reporter: itiel_yn8, Assigned: adw)
References
Details
(Keywords: rtl, Whiteboard: [fxsearch])
Attachments
(1 file)
User Agent: Mozilla/5.0 (Windows NT 10.0; rv:47.0) Gecko/20100101 Firefox/47.0
Build ID: 20160623154057
Steps to reproduce:
1. Install Hebrew version of Firefox Developer Edition 49.0a2 (or possibly any RTL version of Firefox having the new design of the URL bar)
2. a. In the URL bar, type "www.google.com"
b. In the URL bar, type "google." (with a dot after 'google')
3. Observe the first item in the dropdown list, the one having "Visit" after the URL and the second with "Search with Google"
Actual results:
Firefox autocompletes A to "/http://www.google.com", and autocompletes B to ".google" (both being RTL'd)
Notice the '/' and the dot's locations.
Expected results:
A and B should have been "http://www.google.com/" and "google.", respectively.
In short, they should be LTR.
Currently it works okay in 47.0.1 (partially, as the "Search with Google" string is incorrectly LTR'd, which is fixed in the new design of the URL bar).
bug 1267355 is somehow related.
That's still an issue in latest builds.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Updated•8 years ago
|
Priority: -- → P2
Whiteboard: [fxsearch]
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → adw
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Assignee | ||
Comment 3•8 years ago
|
||
Thanks for filing.
This fixes www.google.com but not "google.". It looks like the latter has not been recognized as a URL in the urlbar since Firefox 41, due to this changeset (bug 1107883): https://hg.mozilla.org/mozilla-central/rev/e67ddc64b010#l4.50
The problem is that fixupInfo.keywordAsSent is "google.", so _matchUnknownUrl returns early and does not add a visiturl match for it. That seems like an intentional change made by that bug.
Marco reviewed that patch though, so maybe he can comment on that part.
(In reply to Drew Willcoxon :adw from comment #3)
> This fixes www.google.com but not "google.". It looks like the latter has
> not been recognized as a URL in the urlbar since Firefox 41
That.. actually makes sense. Now that I re-think about it, Firefox shouldn't assume that "google." is a start of a URL.
If the attached patch fixes "www.google.com/", I'm good with closing this bug as RESOLVED FIXED at this point.
Thank you!
Assignee | ||
Comment 5•8 years ago
|
||
All right, thanks for your comment. Is it possible for you to try the following build in an RTL language so you can verify that the patch works?
https://archive.mozilla.org/pub/firefox/try-builds/dwillcoxon@mozilla.com-d6024e03410676c51536ffec8e8e35917be40774/
(Depending on how quickly you check this link after I posted it, it may not have been created yet, or the builds may not have finished yet.)
(In reply to Drew Willcoxon :adw from comment #5)
> All right, thanks for your comment. Is it possible for you to try the
> following build in an RTL language so you can verify that the patch works?
>
> https://archive.mozilla.org/pub/firefox/try-builds/dwillcoxon@mozilla.com-
> d6024e03410676c51536ffec8e8e35917be40774/
It's a bit of a problem to verify whether a RTL issue is fixed or not on a forced-RTL build using the Force-RTL addon. I've seen bugs where the addon forced-RTL the UI anyway, when the issue wasn't actually fixed.
Comment 7•8 years ago
|
||
mozreview-review |
Comment on attachment 8801468 [details]
Bug 1290490 - Firefox shouldn't RTL the first item in the URL dropdown list when typing a URL.
https://reviewboard.mozilla.org/r/86220/#review85566
off-hand looks like it should work.
::: browser/base/content/browser.css:462
(Diff revision 1)
> -moz-binding: url(chrome://browser/content/urlbarBindings.xml#urlbar);
> }
>
> /* Always show URLs LTR. */
> -.ac-url-text:-moz-locale-dir(rtl) {
> +.ac-url-text:-moz-locale-dir(rtl),
> +.ac-title-text[isurl]:-moz-locale-dir(rtl) {
nit: isurl is quite strong and we won't ever be 100% sure, I'd name it lookslikeurl just in case.
::: toolkit/content/widgets/autocomplete.xml:1860
(Diff revision 1)
> // the popup is open.
> this._removeMaxWidths();
> }
>
> let title = this.getAttribute("title");
> + let isTitleUrl = false;
...and titleLooksLikeUrl
Attachment #8801468 -
Flags: review?(mak77) → review+
Comment hidden (mozreview-request) |
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3ce14ff1acd5
Firefox shouldn't RTL the first item in the URL dropdown list when typing a URL. r=mak
Comment 10•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
You need to log in
before you can comment on or make changes to this bug.
Description
•