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)

47 Branch
defect

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.
Component: Untriaged → Location Bar
Keywords: rtl
Has STR: --- → yes
That's still an issue in latest builds.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P2
Whiteboard: [fxsearch]
Assignee: nobody → adw
Status: NEW → ASSIGNED
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!
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 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+
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
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Blocks: 1299691
Blocks: 1534939
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: