Closed Bug 1577774 Opened 2 years ago Closed 5 months ago

A hint message should be displayed when the "Website address" field is focused

Categories

(Firefox :: about:logins, enhancement, P3)

enhancement

Tracking

()

VERIFIED FIXED
86 Branch
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox68 --- unaffected
firefox69 --- disabled
firefox70 --- wontfix
firefox86 --- verified

People

(Reporter: cmuntean, Assigned: simon.weckler)

References

Details

Attachments

(2 files)

Attached image hint message.png

[Notes]:

  • Based on the mock-up a hint message is displayed when the "Website address" field is focused.

[Affected Versions]:

  • Nightly 70.0a1

[Affected Platforms]

  • All Windows
  • All Mac
  • All Linux

[Steps to reproduce]:

  1. Open the latest Nightly browser and navigate to "about:logins" page.
  2. Click the "Create New Login" button.
  3. Observe the "Website address" field.

[Expected results]:

  • The "Website address" field is focused and a hint message is displayed.

[Actual results]:

  • The "Website address" field is focused but no hint message is displayed.

[Notes]:

  • Attached a screenshot with the behavior from mock-ups.
Component: Password Manager → about:logins
Product: Toolkit → Firefox
Version: 70 Branch → unspecified

Mass removing [skyline] and [passwords:management] from about:logins bugs which are no longer useful.

Whiteboard: [passwords:management] [skyline]

Ryan, what should we put in a popup here? Our code will already add "https://" to the beginning of the address if the user doesn't add it themselves. The "www" prefix is not always wanted.

Flags: needinfo?(rgaddis)

Hmmmmm, perhaps this approach is no longer necessary if we are auto-adding the "https://" (that wasn't the case when we originally mocked this up)

What do you think Katie?

Flags: needinfo?(rgaddis) → needinfo?(kcaldwell)

Chatted with Betsy, here's our new and improved string:
"Make sure this matches the exact address of the website where you log in."

Flags: needinfo?(kcaldwell)

I am new to contributing to firefox, but this bug is something i would like to work on in case the hint-message is still needed.

Flags: needinfo?(cmuntean)

Hi Simon, I think that Katie and Tim might be the right persons we can ask if we still want to add the warning.

Flags: needinfo?(tgiles)
Flags: needinfo?(kcaldwell)
Flags: needinfo?(cmuntean)

Having the hint-message would be great! See Comment 11 for the string.

I do have a few questions... for the about:logins team (Tim/Sam)?
• I'm curious if there is any telemetry for this input field / do we know the number of urls added that are broken/invalid?
• is it possible to know (and prevent?) invalid urls? Are we (or are we able to be) checking or autosuggesting for the user in the moment?

Flags: needinfo?(kcaldwell)

Hey Katie, I'll try my best to answer your questions.

I don't immediately see any telemetry regarding the "website" input field.

I'm also unsure what you mean by "number of urls added that are broken/invalid." The password manager doesn't check to see if the website that was entered goes to a valid webpage or anything (and I'm not sure how we would accomplish that off the top of my head). I.e. I can enter "g" for the website with some fake username and password and the password manager will save this into the management UI. This will be saved as "https://g" for the website address in this case.

However, the password manager does check to make sure the website origin is valid. I.e. I can enter "g" for the website and the password manager will add the entry as "https://g" (not that this example goes to a valid website) but if I enter "qwerty://g" for the website, the password manager will not add this entry but does not explain to the user why this action failed (there is an error thrown in the console about "Unable to get an origin from the login details." since qwerty is not a valid origin). However, the password manager will inform the user that an entry of just "http://" or "https://" is invalid.

Given this information, I'm not sure of any way to help the user regarding autosuggestions, but we could inform the user that the particular origin (like qwerty://, abc://) is invalid since we're already capturing that error anyway. I'm not sure how valid this case is since qwerty://www.somesite.com will never be a valid webpage...at any rate, hope that answers your questions Katie! let me know if you have any other questions

Flags: needinfo?(tgiles) → needinfo?(kcaldwell)

Thanks for the reply Tim!

regarding: "number of urls added that are broken/invalid."
you probably didn't follow this question because I wasn't clear enough. let me try again. :)

If someone has a saved login with a bad/invalid url (when clicked will give an error, "Hmm. We’re having trouble finding that site. We can’t connect to the server at https://g") when they click the saved website address from about:logins and they land on an error page, do we know how often this happens (... can we identify if this is even an issue)?

"does not explain to the user why this action failed... "
We need to/good ux practices always clearly explain errors to people (so they understand and can confidently take action to fix the error), which is why updating the hint-message has value and is important.

Flags: needinfo?(kcaldwell)

(In reply to katieC from comment #9)

If someone has a saved login with a bad/invalid url (when clicked will give an error, "Hmm. We’re having trouble finding that site. We can’t connect to the server at https://g") when they click the saved website address from about:logins and they land on an error page, do we know how often this happens (... can we identify if this is even an issue)?

I don't think we know when this happens and I don't think we can identify if the website address the user enters will end up resulting in an error page...especially since the "Hmm we're having trouble finding that site" error could happen on a legitimate url, say if a website is temporarily unavailable due to internet issues, site has temporarily moved, and probably other situations.

"does not explain to the user why this action failed... "
We need to/good ux practices always clearly explain errors to people (so they understand and can confidently take action to fix the error), which is why updating the hint-message has value and is important.

Agreed on this. @Simon, if you're still interested in contributing to this bug, let me know and I can assign this to you! Feel free to submit a patch as well, which will automatically assign you to the bug. If you need help getting started, let me know and I'll provide additional resources to help out. This MDN developer guide should help out regardless!

Flags: needinfo?(simon.weckler)

Yes, I am still interested. I would now add the necessary code to the aboutLogins.html and the login-item.css file in browser\components\aboutlogins\content\components (or is there a better approach you would recommend? ).
Temporarely i am going to add the text, katieC and Betsy agreed on, to the html file, but obvisiously that's a bad practice so I would like to now how you handle strings resources in about:logins.

Flags: needinfo?(simon.weckler) → needinfo?(tgiles)

(In reply to Simon Weckler from comment #11)

Yes, I am still interested.

Perfect! I'll go ahead and assign you the bug.

I would now add the necessary code to the aboutLogins.html and the login-item.css file in browser\components\aboutlogins\content\components (or is there a better approach you would recommend? ).

Sounds like a correct path to me!

Temporarely i am going to add the text, katieC and Betsy agreed on, to the html file, but obvisiously that's a bad practice so I would like to now how you handle strings resources in about:logins.

In order to deal with the strings in this case, you will want to add a data-l10n-id to your tooltip div (you'll see examples of other data-l10n-id in the aboutLogins.html file) and then add the created ID to aboutLogins.ftl in browser/locales/en-US/browser/aboutLogins.ftl with a value of "Make sure this matches the exact address of the website where you log in."

Let me know if you have questions!

Assignee: nobody → simon.weckler
Status: NEW → ASSIGNED
Flags: needinfo?(tgiles)

Ok, thank you very much.

Hi Simon, I'd hate to do this after you've submitted a patch but we've hit a fork in the road. Given that there is existing form validation on the website and password fields when creating a new login:

  1. This patch is dropped due to priority shift and the impending UI refresh
  2. This patch's scope is increased to drop the newly implemented tool tip and instead use custom form validation for the website field.
    • This would using the existing form validation that appears under the website field. The form validation's string would change based on the state of the field. The validation check would be tricky however, since we would want the "Make sure this matches the exact..." to appear when the form is first focused (not checking if the contents of the field are valid since it's the first time the user has focused the field), but not appear if the website field is submitted with an invalid input and the contents are invalid...but then the "Make sure" string would probably need to reappear when fixing the website input.
    • You would need to use constraint validation and/or the constraint validation API, so if you have previous experience writing custom validation...that would reduce the lift needed significantly in my opinion.

I'd hate to stall/prevent a contribution like this, but unfortunately priorities shift. Please let me know which path you'd like to take with this.

Flags: needinfo?(simon.weckler)

Please correct me if I understood something wrong or made any mistakes.

Ok, I tried to implement what you suggested, but there were a few problems i came across (maybe because my approach was not the right one) :

1

In order set a custom validation message you have to use the setCustomValidity(msg) function and in order to actually show the message the reportValidity() function must be called. The problem is: once called, reportValidity() sets invalid to true --> the styling of the input field changes as if something was wrong with the input.
However, this can be prevented by immediatley calling setCustomValidity("") after reportValidity().

2

The next problem i encountered was, that i was unable to implement the conditions you mentioned above. (Of course this does not mean it is not possible, i only want to express that it is rather hard to do so)

3

The third problem, was that as soon you type something in and firefox gives you suggestions, the validation box is not fully visible anymore. (See Image)

msg convered by browser suggestions

Conclusion:

Because of the problems i came across (especially the third), i suggest that instead of displaying the tooltip message in another box i would apply the conditions you mentioned (only showing it when the user first clicks on the input and if the input is wrong and the user now wants to edit it.) to the newly implemented tooltip message. An advantage is, that this can be done only using css.

Please let me know if this is reasonable/makes sense to you.

Flags: needinfo?(simon.weckler) → needinfo?(tgiles)

Hey Simon, thanks for doing all this, I appreciate it! I think the message being covered by the browser suggestions is definitely a deal-breaker, so let's not worry about getting the "Make sure this matches..." under the website field. I think your conclusion is valid and sound and so I'll review your patch if you want to implement those changes!

Thanks for your patience and work!

Flags: needinfo?(tgiles)

Thank you !
i implemented the changes and submitted them.

Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ae0a510348cd
added tooltip message for the url input field. r=tgiles,fluent-reviewers,flod
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 86 Branch

I have verified this on the latest Nightly 86.0a1 (Build ID 20210119091250) on Windows 10 x64, macOS 10.15.6 and Linux Mint 20.

  • The hint message is correctly displayed when the "Website address" field is focused.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.