When adding "Copy link" to the address bar, "Copy link" doesn't have any feedback when the link has been copied

VERIFIED FIXED in Firefox 57

Status

()

P1
normal
VERIFIED FIXED
2 years ago
6 months ago

People

(Reporter: ntim, Assigned: jaws)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

unspecified
Firefox 57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

(Whiteboard: [reserve-photon-animation])

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

2 years ago
STR:
- Open page action menu
- Right click on "Copy Link"
- Click on "Add to address bar"
- Click on the "Copy Link" button in the address bar.


AR:
You have absolutely no indication that the link has been copied.

ER:
I would expect an animation, or a panel saying the link has been copied

Updated

2 years ago
Blocks: 1352697
No longer blocks: 1349210
Summary: When adding "Copy link" to the address bar, "Copy link" doesn't have any feedback when the link has been copied → [UX] When adding "Copy link" to the address bar, "Copy link" doesn't have any feedback when the link has been copied
Whiteboard: [photon-structure][triage][photon-animation][triage]
Bryan / Stephen, what do you think we should do here?
Flags: needinfo?(shorlander)
Flags: needinfo?(bbell)
(Reporter)

Comment 2

2 years ago
I think it would help if the icons had a hover/active state like described in the spec. Right now, the button looks disabled.
Aaron had some ideas for this.
Flags: needinfo?(shorlander) → needinfo?(abenson)
Comment hidden (typo)

Updated

2 years ago
Flags: needinfo?(bbell)

Comment 5

2 years ago
In thinking about this some more, the animation probably shouldn't be dependent on the three dots since the "Copy URL" action can be placed inside the URL bar (which was the point of this bug but I misread it a little). In my comments above I was only thinking of how it would behave if you clicked on "Copy URL" from the Page Action Menu.

In either case, a simple animation with a checkmark to indicate 'task completed' would be my recommendation .. and working in the same "pop" effect that the bookmark icon has would be a nice way to carry on that animation element.
Whiteboard: [photon-structure][triage][photon-animation][triage] → [photon-animation][triage]
Flags: qe-verify-
Priority: -- → P3
Whiteboard: [photon-animation][triage] → [reserve-photon-animation]
Assignee: nobody → amlee
Status: NEW → ASSIGNED
Iteration: --- → 57.1 - Aug 15
Priority: P3 → P1
Whiteboard: [reserve-photon-animation] → [reserve-photon-animation] [ux]
Iteration: 57.1 - Aug 15 → 57.2 - Aug 29
Link to spec for Copy URL confirmation message 

https://mozilla.invisionapp.com/share/CFD5CY9KW
Flags: needinfo?(amlee)
Assignee: amlee → jaws
Posted video Copy URL .mp4 (obsolete) —
Flags: qe-verify- → qe-verify?
Summary: [UX] When adding "Copy link" to the address bar, "Copy link" doesn't have any feedback when the link has been copied → When adding "Copy link" to the address bar, "Copy link" doesn't have any feedback when the link has been copied
Whiteboard: [reserve-photon-animation] [ux] → [reserve-photon-animation]
Assignee: jaws → nobody
Status: ASSIGNED → NEW
Flags: qe-verify? → qe-verify+
Priority: P1 → P3
QA Contact: stefan.georgiev
Iteration: 57.2 - Aug 29 → ---
Posted image checkmark.svg
Iteration: --- → 57.2 - Aug 29
Priority: P3 → P1
Comment hidden (mozreview-request)
Hey David, in this bug we're adding a transient popup notification next to the "Copy URL" button in the location bar. The popup notification will say "Copied!" when the user clicks on and the current page's URL is put in their clipboard.

The popup has role="alert" on it so as I understand it screenreaders should handle it correctly. The popup will autoclose on its own or can close if the user clicks elsewhere. Right now we have it implemented to close after 1 second of being open since the alert text is so terse. Is there a general guideline of how much time is acceptable for users to have enough time to read something like this?

If you would like to try it out, here's a link to a trypush:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cebc19ea381bc70db7a4a6423a78678f153b3ac2&selectedJob=125762766

You can make the popup stay open longer by adding an Integer pref to about:config with the name of "browser.pageActions.feedbackTimeoutMS", setting the value to the number of milliseconds the popup should stay open. It defaults to 1120ms if the pref doesn't exist.
Flags: needinfo?(dbolter)
Comment on attachment 8900918 [details]
Bug 1387042 - Add a toast notification that will be shown when the Copy Link or Send to Device page actions are used.

Hey Drew, can you please review the changes to Page Actions?
Attachment #8900918 - Flags: review?(adw)

Comment 14

2 years ago
mozreview-review
Comment on attachment 8900918 [details]
Bug 1387042 - Add a toast notification that will be shown when the Copy Link or Send to Device page actions are used.

https://reviewboard.mozilla.org/r/172354/#review178140

I'll attach a screenshot of the popup on Ubuntu. I'm not sure what is going there - possibly an artifact of the larger default font-size there, or Ubuntu not knowing how to draw a popup that small? Otherwise, with the comments below this looks good.

::: browser/base/content/browser-pageActions.js:723
(Diff revision 2)
> +      this.feedbackAnimationBox.removeAttribute("animate");
> +    }, {once: true});
> +
> +    // The timeout value used here allows the panel to stay open for
> +    // 1 second after the text transition (duration=120ms) has finished.
> +    setTimeout(() => {

Maybe use requestAnimationFrame inside the setTimeout callback here? See thread around https://bugzilla.mozilla.org/show_bug.cgi?id=1388183#c13

::: browser/themes/shared/icons/check-animation.svg:7
(Diff revision 2)
> +   - License, v. 2.0. If a copy of the MPL was not distributed with this
> +   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> +<svg xmlns="http://www.w3.org/2000/svg" width="266" height="14">
> +  <svg>
> +    <defs>
> +      <mask id="a">

Can you file a follow-up bug optimize this SVG? 
Looks like some of these masks use the same path, and I think we may be able to bake the translate adjustments into the paths for less runtime computation
Attachment #8900918 - Flags: review?(sfoster)
(Reporter)

Comment 15

2 years ago
mozreview-review
Comment on attachment 8900918 [details]
Bug 1387042 - Add a toast notification that will be shown when the Copy Link or Send to Device page actions are used.

https://reviewboard.mozilla.org/r/172354/#review178172

::: browser/themes/shared/icons/check-animation.svg:5
(Diff revision 2)
> +<!-- This Source Code Form is subject to the terms of the Mozilla Public
> +   - License, v. 2.0. If a copy of the MPL was not distributed with this
> +   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> +<svg xmlns="http://www.w3.org/2000/svg" width="266" height="14">
> +  <svg>

This first element is 100% masked, so you can just remove it.

::: browser/themes/shared/icons/check-animation.svg:157
(Diff revision 2)
> +        <path fill="#fff" stroke="#fff" stroke-width="30" d="M-2.91 5.508v7.07h4.488v-7.07H-2.91"/>
> +      </mask>
> +    </defs>
> +    <path fill="#FFF" d="M6 14a.997.997 0 0 1-.707-.293l-3-3a1 1 0 0 1 1.414-1.414l2.157 2.157 6.316-9.023a1 1 0 0 1 1.64 1.146l-7 10a1 1 0 0 1-.733.427A1.262 1.262 0 0 1 6 14z" mask="url(#s)" transform="translate(-.94 -.94)"/>
> +  </svg>
> +  <svg x="266">

This element is offscreen from the SVG, you can remove it.
Thanks! MarcoZ can you try the build in comment 12?
Flags: needinfo?(dbolter) → needinfo?(mzehe)
Comment hidden (mozreview-request)
Comment on attachment 8900918 [details]
Bug 1387042 - Add a toast notification that will be shown when the Copy Link or Send to Device page actions are used.

https://reviewboard.mozilla.org/r/172354/#review178140

> Maybe use requestAnimationFrame inside the setTimeout callback here? See thread around https://bugzilla.mozilla.org/show_bug.cgi?id=1388183#c13

I read through the logs since that comment doesn't explain enough of the conversation. This doesn't seem related to what we're doing here which is just hiding the popup. The discussion that this was about was dealing with back to back animations, and making sure there is a style flush between them, which this doesn't really need to care about.

Comment 19

2 years ago
mozreview-review
Comment on attachment 8900918 [details]
Bug 1387042 - Add a toast notification that will be shown when the Copy Link or Send to Device page actions are used.

https://reviewboard.mozilla.org/r/172354/#review178262

Nice!  Do you think this could be extended to support other animated feedback, like we have on the star and Pocket buttons, kind of like we discussed in IRC the other day?  Just curious if you thought about that, no worries.

::: browser/base/content/browser-pageActions.js:701
(Diff revision 3)
> +  show(action, event) {
> +    this.feedbackLabel.textContent = this.panelNode.getAttribute(action + "Feedback");
> +    this.panelNode.hidden = false;
> +
> +    let anchor = BrowserPageActions.mainButtonNode;
> +    if (event.target.classList.contains("urlbar-icon")) {

Will this be OK for actions with wrappers?  I'm sure you've thought of that, just want to double check.
Attachment #8900918 - Flags: review?(adw) → review+
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #18)
> > Maybe use requestAnimationFrame inside the setTimeout callback here? See thread around https://bugzilla.mozilla.org/show_bug.cgi?id=1388183#c13
> 
> I read through the logs since that comment doesn't explain enough of the
> conversation. This doesn't seem related to what we're doing here which is
> just hiding the popup. The discussion that this was about was dealing with
> back to back animations, and making sure there is a style flush between
> them, which this doesn't really need to care about.

My understanding is that the point of the requestAnimationFrame is that setTimeout is a blunt tool for scheduling operations which will touch the DOM, cause reflow and paint etc. rAF allows this to be scheduled for a more optimal time. I'm not sure how applicable this is here though, and its a easy fix if it turns out to be a good idea

Comment 21

2 years ago
mozreview-review
Comment on attachment 8900918 [details]
Bug 1387042 - Add a toast notification that will be shown when the Copy Link or Send to Device page actions are used.

https://reviewboard.mozilla.org/r/172354/#review178388

Looks good!

::: browser/themes/linux/customizableui/panelUI.css:96
(Diff revision 3)
>  
> +/*
> + * #pageActionFeedbackAnimatableImage is wider than the panel, and due to a
> + * bug in panels on Linux, a box-shadow appears where the image would be if
> + * overflow:hidden wasn't applied. Disabling the box-shadow for this panel on
> + * Linux works around this issue. This bug is on file as XXXXXX.

Nit: update comment with the bug number
Attachment #8900918 - Flags: review?(sfoster) → review+
Comment on attachment 8900918 [details]
Bug 1387042 - Add a toast notification that will be shown when the Copy Link or Send to Device page actions are used.

https://reviewboard.mozilla.org/r/172354/#review178262

> Will this be OK for actions with wrappers?  I'm sure you've thought of that, just want to double check.

Yes, I tested this with the bookmark star and the event target is the `.urlbar-icon`. However if the star is already filled in then the event.target is the #star-button-animatable-image and so it doesn't have the `urlbar-icon` class. Right now we only want to support this for the Copy URL and Send Page to Device, but we could extend this to other page action buttons easily without too much extra work.
Comment hidden (mozreview-request)

Comment 24

2 years ago
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3417ead48c7f
Add a toast notification that will be shown when the Copy Link or Send to Device page actions are used. r=adw,sfoster
Iteration: 57.2 - Aug 29 → 57.3 - Sep 19
Will test this in Nightly when it lands there, and file a followup if necessary. The try builds didn't appear for me for Windows for some reason. Role="alert" should normally do the trick just fine. It's an implicit aria-live="assertive" and is normally picked up by screen readers, like with doorhangers. Clearing NI.
Flags: needinfo?(mzehe)
https://hg.mozilla.org/mozilla-central/rev/3417ead48c7f
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57

Comment 27

2 years ago
I have reproduced this bug with Nightly 57.0a1 (2017-08-03) on Windows 8.1, 64 Bit!

This bug's fix is verified on Latest Nightly 57.0a1

Build ID : 20170830100230
User Agent : Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
QA Whiteboard: [bugday-20170830]

Updated

2 years ago
Blocks: 1395154
Status: RESOLVED → VERIFIED
Flags: qe-verify+
I can also verify that screen readers speak the message "Copied" when the URL has been copied.

Updated

2 years ago
Depends on: 1395398

Updated

2 years ago
Depends on: 1395410
See Also: → bug 1398681

Updated

2 years ago
See Also: bug 1398681
Depends on: 1471558
No longer depends on: 1471558
You need to log in before you can comment on or make changes to this bug.