Unable to type capital L as the first character in the Awesome Bar

VERIFIED FIXED in Firefox 62

Status

()

defect
P1
normal
VERIFIED FIXED
Last year
11 months ago

People

(Reporter: rkrueger11, Assigned: adw)

Tracking

({regression})

Trunk
Firefox 63
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox-esr60 unaffected, firefox61 unaffected, firefox62+ verified, firefox63 verified)

Details

(Whiteboard: [fxsearch][STR for QA in comment 15])

Attachments

(2 attachments)

Reporter

Description

Last year
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:62.0) Gecko/20100101 Firefox/62.0
Build ID: 20180621152331

Steps to reproduce:

Typed a capital L as the first keystroke in the awesome bar


Actual results:

A lowercase "l" appeared with the autocomplete results for "localhost:8888"


Expected results:

A capital "L" should appear along with the same autocomplete results (which are not case sensitive).  This would be consistent with the behavior when I type other capital letters as the first keystroke.

Updated

Last year
Component: Untriaged → Address Bar
hm, this is strange, we should be respecting the casing and I cannot reproduce on Windows.
Assignee

Comment 2

Last year
I can reproduce it (on a new profile).  Probably a regression from my autofill work.  I'll try to verify that.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee

Comment 3

Last year
I'm on a Mac too btw, like comment 0 indicates.  It would be weird if this reproduced on Mac but not Windows, right?
Assignee

Comment 4

Last year
Yeah, this doesn't reproduce on the 5/14/2018 Nightly, which is the one before the autofill bug landed.  We/I should probably try to fix this soon.  I probably removed a case check or splice in UnifiedComplete.js that I shouldn't have, so hopefully this isn't too hard to fix.

Thanks for filing this bug.
Blocks: 1239708
Priority: -- → P1
Reporter

Comment 5

Last year
Of course, happy to help.  Thank you all for doing what you do.
Assignee

Updated

Last year
Assignee: nobody → adw
Status: NEW → ASSIGNED
I just gave this a try in nightly 63 and it still reproduces.

Updated

11 months ago
Whiteboard: [fxsearch]
Assignee

Comment 7

11 months ago
It's taken me a while to track this down.  I'm still not sure where the problem is happening, but it seems to happen only for origins with ports.  I can't reproduce it with example.com, or when localhost is bound to the standard port 80 and I therefore don't need to include a port.

Still investigating.
Assignee

Comment 8

11 months ago
My hunch is that the problem is in nsAutoCompleteController.  We only started autofilling ports at all with the new autofill (see bug 764062).  Previously, in this bug's case, you would only get "Localhost" autofilled, no port.  It's plausible nsAutoCompleteController just isn't handling this case correctly now that it's been exposed.

So in that sense this isn't a regression at all, but it's probably better to think of it as one since from the user's POV, you get the behavior described in comment 0, and it didn't use to happen.
Assignee

Comment 11

11 months ago
The problem is in the urlbar's onBeforeTextValueSet.  When autofill fills an origin without a port (or a scheme), the makeURI call in onBeforeTextValueSet returns null.  The string passed to makeURI in that case is something like "example.com/".  Therefore onBeforeTextValueSet doesn't call losslessDecodeURI on the string, so the string is used as-is for the input value, preserving the case in whatever the user typed.

But when makeURI does return a URI, we call losslessDecodeURI on it, which effectively downcases the string.  makeURI returns a URI for strings like "example.com:8888/".  It also returns a URI if you include the scheme, so this bug can also be reproduced if you visited localhost without a port and type "http://L", which gets autofilled to "http://localhost/".

I modified the base autocomplete binding so that it doesn't call onBeforeTextValueSet if the value is being set due to autofill.  We already had a _disableTrim property to handle a similar case: when _disableTrim is true, we don't call trimValue when `value` is set.  I renamed _disableTrim to _textValueBeingSetDueToCompleteDefault.

It seems OK to not call onBeforeTextValueSet in this case for all autocomplete implementations, not only the urlbar.  IMO it makes sense because a value is being autofilled.  But if you disagree, then we could move the `if (this._textValueBeingSetDueToCompleteDefault)` check directly to urlbar's onBeforeTextValueSet implementation.

The patch also adds test tasks to test_autofill_origins.js.  That test doesn't actually hit this bug since it's an xpcshell test so it's not using the browser's urlbar binding, but it seems like a good idea anyway.
Comment on attachment 8993541 [details]
Bug 1470887 - Preserve exactly autofilled values in the urlbar, and don't call losslessDecodeURI on them. r?mak

Marco Bonardo [::mak] has approved the revision.

https://phabricator.services.mozilla.com/D2256
Attachment #8993541 - Flags: review+

Comment 14

11 months ago
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/22005ac8d9b2
Preserve exactly autofilled values in the urlbar, and don't call losslessDecodeURI on them. r=mak
Assignee

Updated

11 months ago
Flags: qe-verify+
Assignee

Comment 15

11 months ago
STR for QA:

1. Start with a new profile
2. Start typing "mozilla" and verify it's autofilled to "mozilla.org/"
3. Start typing "MOZILLA" (all uppercase) and verify it's autofilled to "mozilla.org/" except it doesn't change the uppercase letters you've typed.  For example, when you type "M", it should be autofilled to "Mozilla.org/".  When you type "MOZ", it should be autofilled to "MOZilla.org/".
4. Start typing "https://mozilla.org" and verify it's autofilled to "http://mozilla.org/"
5. Start typing "https://MOZILLA" and verify it's autofilled to "https://mozilla.org/" except it doesn't change the uppercase letters you've typed, same as in step 3
Whiteboard: [fxsearch] → [fxsearch][STR for QA in comment 15]

Comment 16

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/22005ac8d9b2
Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Assignee

Comment 17

11 months ago
Camelia, could you please verify this bug on Nightly and this Beta try build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5701164694dc6a9368a29b4f54384daa6fbf7ff4

STR are in comment 15.  Thank you!

I'll request uplift after verification.
Flags: needinfo?(camelia.badau)
Assignee

Comment 18

11 months ago
Looks like there are unrelated build failures on macOS right now, bug 1477408.  I'll have to trigger new builds on macOS whenever that's fixed.
Assignee

Comment 20

11 months ago
Posted patch Beta/62 patchSplinter Review

Updated

11 months ago
Flags: needinfo?(mak77)
I've tested on Windows 7 x64, Ubuntu 16.04 x64 and macOS 10.13 using latest Nightly 63.0a1 (2018-07-23) and Beta try build from comment 17 (comment 19 - for mac) and everything looks fine according to STR from comment 15.
Flags: needinfo?(camelia.badau)
Comment on attachment 8993887 [details] [diff] [review]
Beta/62 patch

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1239708 
[User impact if declined]: When the user is typing in the address bar, we may not respect the typed casing, preventing the user to type the expected string
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: already provided
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: it's skipping one case where we were not expected to transform the input text
[String changes made/needed]: none
Flags: needinfo?(mak77)
Attachment #8993887 - Flags: approval-mozilla-beta?
Status: RESOLVED → VERIFIED
Comment on attachment 8993887 [details] [diff] [review]
Beta/62 patch

Address bar fix for a recent regression in 62, verified in nightly, let's uplift for beta 13.
Attachment #8993887 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
I have managed to reproduce this issue by following the steps provided in comment 15 using Firefox 63.0a1 (BuildId:20180625220119).

This issue is verified fixed using Firefox 62.0b12 (BuildId:20180726080725) available on treeherder, on Windows 10 64bit, macOS 10.13.4 and Ubuntu 16.04 64bit.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.