Refine footer banner in about:feedback

RESOLVED FIXED in Firefox 36

Status

()

Firefox for Android
General
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: vivek, Assigned: vivek, Mentored)

Tracking

unspecified
Firefox 36
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [lang=html][lang=js][lang=css])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 years ago
This a follow up bug for the comment https://bugzilla.mozilla.org/show_bug.cgi?id=1007436#c74

As mentioned there, entire footer must be clickable and support page must be opened  in new tab
(Assignee)

Updated

3 years ago
Assignee: nobody → vivekb.balakrishnan
Depends on: 1007436
Whiteboard: [lang=html][lang=js][lang=css]
(Assignee)

Comment 1

3 years ago
Created attachment 8502798 [details] [diff] [review]
1077047.patch

A small patch that makes footer clickable and opens the support page in new tab.
Attachment #8502798 - Flags: review?(margaret.leibovic)

Comment 2

3 years ago
Comment on attachment 8502798 [details] [diff] [review]
1077047.patch

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

Looking good, I just have a few small pieces of feedback.

::: mobile/android/chrome/content/aboutFeedback.js
@@ +67,5 @@
>    }
>  
>    let sumoLink = Services.urlFormatter.formatURLPref("app.support.baseURL");
> +  document.querySelector('footer').addEventListener("click", function() {
> +    window.open(sumoLink, '_blank');

Nit: Use double quotes instead of single quotes.

I would also use document.getElementById("help-section") instead of document.querySelector("footer"), so that it's more obvious that we're adding this listener to a help message.

::: mobile/android/chrome/content/aboutFeedback.xhtml
@@ +81,5 @@
>  
>    <footer>
>      <div id="help-section">
>        <img id="sumo-icon" />
> +      <span>&support.pre3;<span id="sumo-link">&support.link2;</span>&support.post3;</span>

Since we don't need to single out this link element anymore, you could replace the id="sumo-link" with class="link", then make the styles apply more generally to that .link class.
Attachment #8502798 - Flags: review?(margaret.leibovic) → feedback+
(Assignee)

Comment 3

3 years ago
Created attachment 8503185 [details] [diff] [review]
1077047.patch

Patch with nits corrected
Attachment #8502798 - Attachment is obsolete: true
Attachment #8503185 - Flags: review?(margaret.leibovic)

Comment 4

3 years ago
Comment on attachment 8503185 [details] [diff] [review]
1077047.patch

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

Nice.
Attachment #8503185 - Flags: review?(margaret.leibovic) → review+

Updated

3 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/b648f59fb34e
Keywords: checkin-needed
Whiteboard: [lang=html][lang=js][lang=css] → [lang=html][lang=js][lang=css][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/b648f59fb34e
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Whiteboard: [lang=html][lang=js][lang=css][fixed-in-fx-team] → [lang=html][lang=js][lang=css]
Target Milestone: --- → Firefox 36
You need to log in before you can comment on or make changes to this bug.