Closed Bug 791662 Opened 7 years ago Closed 7 years ago

Invoke a toaster notification when Copy to clipboard button is tapped in about:support

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 18

People

(Reporter: xti, Assigned: capella)

References

Details

Attachments

(1 file, 2 obsolete files)

Firefox 18.0a1 (2012-09-17)
Device: Galaxy Note
OS: Android 4.0.4

Steps to reproduce:
1. Open Firefox for Android
2. Go to about:support
3. Tap on "Copy to clipboard" button

Expected result:
A toaster notification is triggered as a confirmation that the data was copied.

Actual result:
Nothing happens after step 3. It feels like the button is not working at all.
Duplicate of this bug: 791668
It looks like we'd need to add some #ifdef ANDROID code in abourSupport.js :
http://mxr.mozilla.org/mozilla-central/source/toolkit/content/aboutSupport.js#511

The file already has some other platform preprocessing in it, so it's not the end of the world.
Capella, this sounds like a bug you might be interested in :)
Yeah!
Assignee: nobody → markcapella
Status: NEW → ASSIGNED
Though my device provides haptic feedback here ... I tried to add it in once and found it already present ...
Yeah, we don't need to add haptic feedback, but we'd like a toast notification to show up, similarly to what happens when you copy text using the context menu. See:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#1860
Attached patch Patch (v1) (obsolete) — Splinter Review
Ok, here's what I hacked together :)

(It works) ... Let me know if there's a better / cleaner way ?
Attachment #662790 - Flags: review?(margaret.leibovic)
Comment on attachment 662790 [details] [diff] [review]
Patch (v1)

Using NativeWindow is probably fine, but if you wanted to skip it you could just send the JSON message to Java yourself. Here is what NativeWindow does:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#1153

and sendMessageToJava is here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#89

Also, add the string to aboutSupport.properties and access it using the "stringBundle()" function used in other places in aboutSupport.js
Attached patch Patch (v2) (obsolete) — Splinter Review
Modified to use mfinkles suggestions ...
Attachment #662790 - Attachment is obsolete: true
Attachment #662790 - Flags: review?(margaret.leibovic)
Attachment #662835 - Flags: review?(margaret.leibovic)
Comment on attachment 662835 [details] [diff] [review]
Patch (v2)

Review of attachment 662835 [details] [diff] [review]:
-----------------------------------------------------------------

r+, but please address my comment before landing.

::: toolkit/content/aboutSupport.js
@@ +540,5 @@
> +
> +#ifdef ANDROID
> +  // Present a toast notification.
> +  let bundle = Services.strings.
> +    createBundle("chrome://global/locale/aboutSupport.properties");

mfinkle mentioned this in his comment, but you should use the stringBundle() function that already exists in this file to get this.
Attachment #662835 - Flags: review?(margaret.leibovic) → review+
Well I must be missing something here ... the existing "bundle" doesn't seem to be defined in the function, so I had to re- "createBundle" it ... let me look again ...
Attached patch Patch (v3)Splinter Review
Well, my repo was out of date ... explaining why I couldn't locate that function. I pulled to the current level, and updated the patch.

Also, there's been a new button added to the page, to "copy raw data" to the clipboard that was rendering poorly like the situation we fixed in Bug 774500 - Make about:support more mobile-friendly.

I went ahead CSS-styled it, and added a toast notification there also. Since this is definitely not in the original scope, I'm asking for review again.
Attachment #662835 - Attachment is obsolete: true
Attachment #663268 - Flags: review?
Attachment #663268 - Flags: review? → review?(margaret.leibovic)
Comment on attachment 663268 [details] [diff] [review]
Patch (v3)

Review of attachment 663268 [details] [diff] [review]:
-----------------------------------------------------------------

Good catch on the new button.

::: toolkit/locales/en-US/chrome/global/aboutSupport.properties
@@ +11,5 @@
>  # "GPU Accelerated Windows: 2/2 (Direct3D 9)"
>  # "GPU Accelerated Windows: 0/2"
>  acceleratedWindows = GPU Accelerated Windows
>  
> +# Raw Data Selection

You should put "LOCALIZATION NOTE:" at the front of your comments. Maybe also mention that these strings appear on buttons?
Attachment #663268 - Flags: review?(margaret.leibovic) → review+
https://hg.mozilla.org/mozilla-central/rev/18971061c844
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 18
Verified on:
Firefox Nightly 18.0a1 (2012-09-24)
Device: Galaxy R
OS: Android 2.3.4
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.