1259186 - Send DOM event when feedback page is automatically loaded, instead of custom URL params

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[:Margaret Leibovic]

See bug 1256247 comment 8.

Comment 1

[:Margaret Leibovic]

Attachment: MozReview Request: Bug 1259186 - Send DOM event when feedback page is automatically loaded, instead of custom URL params. r=grisha

Review commit: https://reviewboard.mozilla.org/r/42071/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/42071/

Comment 2

[:Margaret Leibovic]

I tested this locally with this test page:
http://people.mozilla.org/~mleibovic/feedback.html

Justin, here's a build you can use to test this out:
http://people.mozilla.org/~mleibovic/feedback-event.apk

Comment 3

[:Grisha Kruglov]

Comment on attachment 8734062 [details]
MozReview Request: Bug 1259186 - Send DOM event when feedback page is automatically loaded, instead of custom URL params. r=grisha

https://reviewboard.mozilla.org/r/42071/#review39285

Looks good, but see my comment RE your comment :)

::: mobile/android/chrome/content/Feedback.js:30
(Diff revision 1)
>      let browser = BrowserApp.selectOrAddTab(url, { parentId: BrowserApp.selectedTab.id }).browser;
> +
>      browser.addEventListener("FeedbackClose", this, false, true);
>      browser.addEventListener("FeedbackMaybeLater", this, false, true);
> +
> +    // Dispatch a custom event to the page content when feedback is prompted by the browser.

I would perhaps clarify this comment, to explicitly state that this custom event will be used by the page to determine that it's being loaded from fennec's menus, vs from history. It'll be easier to remember what this is for in a couple of months :)

Comment 4

[:Margaret Leibovic]

Just poking this bug... hoosteeno is this still something you're working on? I'd like to verify this before landing.

Comment 5

[Justin Crawford [:hoosteeno] [:jcrawford]]

It deserves a poke. I intend to hack on it during the week of 4/11. Keeping the needinfo as evidence of my good intentions. :)

Comment 6

[Justin Crawford [:hoosteeno] [:jcrawford]]

PR here: https://github.com/mozilla/fjord/pull/737

Comment 7

[:Margaret Leibovic]

https://hg.mozilla.org/integration/fx-team/rev/c7985389034d03f3e13db28dcd31c94912af550a
Bug 1259186 - Send DOM event when feedback page is automatically loaded, instead of custom URL params. r=grisha

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c7985389034d

Comment 9

[Justin Crawford [:hoosteeno] [:jcrawford]]

The web parts of this landed in staging and prod just now: https://bugzilla.mozilla.org/show_bug.cgi?id=1225676

Comment 10

[:Margaret Leibovic]

Comment on attachment 8734062 [details]
MozReview Request: Bug 1259186 - Send DOM event when feedback page is automatically loaded, instead of custom URL params. r=grisha

Approval Request Comment
[Feature/regressing bug #]: There is a change on the server side that requires this change on the client.

[User impact if declined]: "Maybe later" and "No Thanks" are missing from Firefox feedback page (bug 1271910)

[Describe test coverage new/current, TreeHerder]: Baked on Nightly/Aurora

[Risks and why]: Low-risk, only affects feedback page.

[String/UUID change made/needed]: None.

Comment 11

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8734062 [details]
MozReview Request: Bug 1259186 - Send DOM event when feedback page is automatically loaded, instead of custom URL params. r=grisha

Recent regression, Beta47+

Comment 12

[Carsten Book [:Tomcat]]

has problems uplifting to beta:

Tomcats-MacBook-Pro-2:mozilla-central Tomcat$ hg graft --edit -r c7985389034d
grafting 340568:c7985389034d "Bug 1259186 - Send DOM event when feedback page is automatically loaded, instead of custom URL params. r=grisha"
merging mobile/android/chrome/content/Feedback.js
warning: conflicts while merging mobile/android/chrome/content/Feedback.js! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')

Comment 13

[:Margaret Leibovic]

https://hg.mozilla.org/releases/mozilla-beta/rev/ecb67929a43fcc530a454f50015af681d86774b0

Comment 14

[Ioana Chiorean]

As per comment 3 in Bug 1271910