Closed
Bug 1122225
Opened 9 years ago
Closed 9 years ago
Add context menu for about:passwords actions
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox39 verified)
RESOLVED
FIXED
Firefox 39
Tracking | Status | |
---|---|---|
firefox39 | --- | verified |
People
(Reporter: liuche, Assigned: liuche)
References
Details
Attachments
(4 files, 2 obsolete files)
Add some basic functionality to about:passwords items. The context menu opens on single-click. We don't have "Open in app" functionality yet, and "Edit" will be "Details" for the moment. "Reveal" will toggle with "Hide".
Assignee | ||
Comment 1•9 years ago
|
||
This is a mock for the future custom work in about:passwords.
Comment 2•9 years ago
|
||
I believe about:downloads already uses long-tap context menus for row items. You could look at the code and use the same pattern.
Comment 3•9 years ago
|
||
Since we have downloads, apps, add-ons and passwords - it would be nice to get some consistency in the UIs.
Comment 4•9 years ago
|
||
^ Definitely. Most (if not all) elements of this menu are consistent with our overflow menu items too.
Updated•9 years ago
|
Blocks: passwords-2015-Q1
Assignee | ||
Updated•9 years ago
|
Blocks: mobile-about-passwords
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → liuche
Assignee | ||
Comment 5•9 years ago
|
||
Doing this in Java via Prompts would be awkward and involve some ugly hacking for getting the positioning of Android UI elements to be in line with JS content. Gecko has a XUL element menupopup [1] seems like what we're looking for, but I have no experience using XUL on FxA. I think I would need to convert aboutPasswords.xhtml to a XUL file, and then reference that as the uri in AboutRedirector (assuming XUL uris can be handled as well). [1] https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/menupopup
Comment 6•9 years ago
|
||
(In reply to Chenxia Liu [:liuche] from comment #5) > Doing this in Java via Prompts would be awkward and involve some ugly > hacking for getting the positioning of Android UI elements to be in line > with JS content. > > Gecko has a XUL element menupopup [1] seems like what we're looking for, but > I have no experience using XUL on FxA. I think I would need to convert > aboutPasswords.xhtml to a XUL file, and then reference that as the uri in > AboutRedirector (assuming XUL uris can be handled as well). > > [1] https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/menupopup I would caution against using any XUL in the XHTML file. I predict misery and pain. That said, why are we recreating yet another style of context menu? We already have a well established pattern for context menus in FxA: Prompts or HTML contextmenu Both of these create a touch-friendly popup used in other places in the application. Let's not make a new context menu style. aboutDownloads already uses the existing system. Long presses in web content already use the existing system. Do it for the children!
Assignee | ||
Comment 7•9 years ago
|
||
Oh! I would certainly prefer using HTML context menus, but I think my very-basic level of JS/HTML is showing. I see we do use it in about:downloads, though we want to tie the action to a single click vs long-press. That being said, the Prompts API won't work because it uses Android Prompts, and positioning them over webcontent and calculating offsets will be ugly.
Assignee | ||
Comment 8•9 years ago
|
||
I'll look into using html menus, there seem to be a few menu types and maybe also some way to link it up to singlepress vs longpress.
Comment 10•9 years ago
|
||
(In reply to Mark Finkle (:mfinkle) from comment #6) > (In reply to Chenxia Liu [:liuche] from comment #5) > > Doing this in Java via Prompts would be awkward and involve some ugly > > hacking for getting the positioning of Android UI elements to be in line > > with JS content. > > > > Gecko has a XUL element menupopup [1] seems like what we're looking for, but > > I have no experience using XUL on FxA. I think I would need to convert > > aboutPasswords.xhtml to a XUL file, and then reference that as the uri in > > AboutRedirector (assuming XUL uris can be handled as well). > > > > [1] https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/menupopup > > I would caution against using any XUL in the XHTML file. I predict misery > and pain. +1 do not use XUL! Drwaing XUL is not proparly supported in Fennec, so this won't end well. If we need this to look like it's part of the web content, as the mock-up implies, you could always just make something like a fixed position <div> in the HTML that you position correctly with JS/CSS. You can look at the popup in the reader view code as an example of something like this.
Assignee | ||
Comment 11•9 years ago
|
||
/r/5099 - Bug 1122225 - Add context menu for about:passwords actions. r=ally Pull down this commit: hg pull review -r c7456e6abb523d7edee9377a859e9c6a8178c77a
Attachment #8575746 -
Flags: review?(ally)
Assignee | ||
Comment 12•9 years ago
|
||
Talked to Anthony, and he's okay with using the existing Prompts/Android-dialogs for the first pass because of 1) it'll be more work to write something custom, 2) this will be more consistent with the existing about: pages, which use the Android dialogs. I filed bug 1141897 for using webcontent context menus throughout the about: pages.
Updated•9 years ago
|
Attachment #8575746 -
Flags: review?(ally)
Comment 13•9 years ago
|
||
Comment on attachment 8575746 [details] MozReview Request: bz://1122225/liuche https://reviewboard.mozilla.org/r/5097/#review4459 ::: mobile/android/chrome/content/aboutPasswords.js (Diff revision 1) > + copyStringAndToast(login.password, gStringBundle.GetStringFromName("passwordsDetails.passwordCopied")); please add a comment about data.button & the arbitary ordering.
Comment 14•9 years ago
|
||
Comment on attachment 8575746 [details]
MozReview Request: bz://1122225/liuche
it autocleared, r+ with nit.
Attachment #8575746 -
Flags: review+
Assignee | ||
Comment 15•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/00579267379b
Target Milestone: --- → Firefox 39
Backed out in https://hg.mozilla.org/integration/fx-team/rev/e11593719494 for rc1 failures: https://treeherder.mozilla.org/logviewer.html#?job_id=2295293&repo=fx-team
Flags: needinfo?(liuche)
Assignee | ||
Comment 17•9 years ago
|
||
Oops, I forgot that there are tests for the password flow, which this patch changes. Updated those flows, tested locally, and pushed to try just because: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a3851f3d450a
Flags: needinfo?(liuche)
Assignee | ||
Comment 18•9 years ago
|
||
Comment on attachment 8575746 [details] MozReview Request: bz://1122225/liuche /r/5099 - Bug 1122225 - Add context menu for about:passwords actions. r=ally /r/5539 - Bug 1122225 - Update tests for changed flow. r=ally Pull down these commits: hg pull review -r 76290b51fbe6f7a368b6a1c16a9e603645755e70
Attachment #8575746 -
Flags: review+ → review?(ally)
Assignee | ||
Comment 19•9 years ago
|
||
Comment on attachment 8575746 [details] MozReview Request: bz://1122225/liuche /r/5099 - Bug 1122225 - Add context menu for about:passwords actions. r=ally /r/5539 - Bug 1122225 - Update tests for changed flow. r=ally Pull down these commits: hg pull review -r 76290b51fbe6f7a368b6a1c16a9e603645755e70
Comment 20•9 years ago
|
||
https://reviewboard.mozilla.org/r/5539/#review4579
Comment 21•9 years ago
|
||
Comment on attachment 8575746 [details] MozReview Request: bz://1122225/liuche https://reviewboard.mozilla.org/r/5097/#review4581 Ship It!
Attachment #8575746 -
Flags: review?(ally) → review+
Comment 22•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/1ce28de7d4e9 https://hg.mozilla.org/mozilla-central/rev/b66daf0d64cf
Comment 23•9 years ago
|
||
On single tap the context menu is invoked with the following options: "Copy password", "Copy username" and "Details" and work as expected: after choosing "Copy password" a notification is displayed: "Password copied", "Copy username" will display "Username copied" and "Details" will provide: the URL, username, age and 2 buttons: copy username and copy password Tested with: Device: Samsung S5 (Android 4.4) Build: Firefox for Android 39.0a1 (2015-03-24)
Assignee | ||
Comment 24•8 years ago
|
||
Attachment #8575746 -
Attachment is obsolete: true
Attachment #8619140 -
Flags: review+
Attachment #8619141 -
Flags: review+
Assignee | ||
Comment 25•8 years ago
|
||
Assignee | ||
Comment 26•8 years ago
|
||
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•