1156017 - Can't save web site passwords from doorhanger

Status: RESOLVED FIXED

(SeaMonkey :: Passwords & Permissions, defect)

Description

[[e-mail]]

Bug 1143903 added two fields to the password doorhanger to display the password that was being saved. Bug 1145913 then added code to read back the values to allow them to be edited. This means that saving passwords from the doorhanger is now broken, although if you turn doorhangers off then you can still save passwords from the notification bar.

Comment 1

[[e-mail]]

Attachment: Proposed patch

I wasn't sure I wanted to preview the username (and password, sort of) like Firefox does, and I definitely didn't want to edit it (this makes no sense to me at all), but this does mean that not only do I need an element which allows me to set the value attribute but read the value property (I have no idea why this is how it was implemented), but it also needs to work when hidden, which means using an HTML input.

Comment 2

[Philip Chee]

Comment on attachment 8594527 [details] [diff] [review]
Proposed patch

This works, but I suggest wrapping it in a <popupnotification> to make it easier to locate for people porting code from Firefox. e.g.

  <popupnotification id="password-notification" hidden="true">
    <popupnotificationcontent hidden="true">
      <html:input id="password-notification-username"/>
      <html:input id="password-notification-password" type="password"/>
    </popupnotificationcontent>
  </popupnotification>

Comment 3

[[e-mail]]

Attachment: Possible patch

So you'd prefer this version?

Comment 4

[Philip Chee]

Comment on attachment 8596274 [details] [diff] [review]
Possible patch

r=me assuming it works.

Comment 5

[[e-mail]]

Pushed comm-central changeset 0907772807f2.

Comment 6

[[e-mail]]

Comment on attachment 8596274 [details] [diff] [review]
Possible patch

[Approval Request Comment]
Regression caused by (bug #): 1145913
User impact if declined: Password saving broken from doorhanger
Testing completed (on m-c, etc.): Landed on c-c
Risk to taking this patch (and alternatives if risky): Low
String changes made by this patch: None

Comment 7

[[e-mail]]

https://hg.mozilla.org/releases/comm-beta/rev/e3abbf42a482