Closed Bug 1077047 Opened 10 years ago Closed 10 years ago

Refine footer banner in about:feedback

Categories

(Firefox for Android Graveyard :: General, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 36

People

(Reporter: vivek, Assigned: vivek, Mentored)

References

Details

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

Attachments

(1 file, 1 obsolete file)

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: nobody → vivekb.balakrishnan
Depends on: 1007436
Whiteboard: [lang=html][lang=js][lang=css]
Attached patch 1077047.patch (obsolete) — Splinter Review
A small patch that makes footer clickable and opens the support page in new tab.
Attachment #8502798 - Flags: review?(margaret.leibovic)
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+
Attached patch 1077047.patchSplinter Review
Patch with nits corrected
Attachment #8502798 - Attachment is obsolete: true
Attachment #8503185 - Flags: review?(margaret.leibovic)
Comment on attachment 8503185 [details] [diff] [review]
1077047.patch

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

Nice.
Attachment #8503185 - Flags: review?(margaret.leibovic) → review+
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
Closed: 10 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
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.