969045 - Add feedback item in Settings charm

Status: VERIFIED FIXED

(Firefox for Metro Graveyard :: General, defect, P1)

Description

[Sam Foster [:sfoster] (he/him)]

As an interim measure, in order to allow easy metro-specific feedback in early releases we need a new "feedback" item in the Settings charm menu. While the default bookmark (Bug 963688) is ephemeral, this will always be there. When the new feedback UI arrives (Bug 963691) the menu item can be co-opted to show that instead of going direct to input.mozilla.org

Comment 1

[Sam Foster [:sfoster] (he/him)]

In order to avoid adding new strings and allow this to make its way to aurora and beta, we may be able to use http://dxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/baseMenuOverlay.dtd#35

Comment 2

[Marco Mucci [:MarcoM]]

Hey Sam, can you add a point value.

Comment 3

[Sam Foster [:sfoster] (he/him)]

Probably a 2, hopefully a 1

Comment 4

[Sam Foster [:sfoster] (he/him)]

Attachment: Add feedback item to settings charm, using string from baseMenuOverlay.dtd

Comment 5

[Matt Brubeck (:mbrubeck)]

Comment on attachment 8371974 [details] [diff] [review]
Add feedback item to settings charm, using string from baseMenuOverlay.dtd

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

Awesomely hacky!  :)

r=mbrubeck as long as this all gets backed out when we add our own feedback UI.

::: browser/metro/locales/jar.mn
@@ +38,5 @@
>  
>  @AB_CD@.jar:
>  relativesrcdir browser/locales:
>    locale/browser/syncBrand.dtd            (%chrome/browser/syncBrand.dtd)
> +  locale/browser/baseMenuOverlay.dtd      (%chrome/browser/baseMenuOverlay.dtd)

Should probably add a comment that this is temporary hack, since we don't want to add dependencies on these browser strings in the long term.

Comment 6

[Sam Foster [:sfoster] (he/him)]

Attachment: Add feedback item to settings charm, using string from baseMenuOverlay.dtd

Added comment per review, carrying r+

Comment 7

[Sam Foster [:sfoster] (he/him)]

On fx-team: https://hg.mozilla.org/integration/fx-team/rev/010e1b10e1fe

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/010e1b10e1fe

Comment 9

[Kamil Jozwiak [:kjozwiak]]

Went through the following new addition and made sure everything was working, used the following Nightly build:
- http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-02-12-03-02-01-mozilla-central/

- Ensured that "Submit Feedback..." appeared under the "Settings" charm without any issues
- Ensured that taping on "Submit Feedback..." slides in the Navigation App Bar and opens a new tab
- Ensured that the correct website is being opened once "Submit Feedback..." is taped under the "Settings" charm
- Ensured that "Submit Feedback..." works correctly under several other combinations of snapped view
- Ensured that the new item was placed in an intuitive order (e.g. not at the top, or underneath Options)
- Ensured that all the other flyouts under the "Settings" charm are working correctly

Comment 10

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8372378 [details] [diff] [review]
Add feedback item to settings charm, using string from baseMenuOverlay.dtd

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Interim workaround for 963691
User impact if declined: No means to submit feedback without manually typing in to the url bar
Testing completed (on m-c, etc.): tested
Risk to taking this patch (and alternatives if risky): low risk, metro-only patch
String or IDL/UUID changes made by this patch: none

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/3b7e60dc640c

Comment 12

[Kamil Jozwiak [:kjozwiak]]

Went through the following verification process with the latest Nightly and Aurora builds:
- http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-02-18-03-02-02-mozilla-central/
- http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-02-18-00-40-01-mozilla-aurora/

- Went through the test cases that have been added in comment #9 without any issues
- Ensured that the "Feedback" website is being placed in the correct order under the Navigation App Bar

Comment 13

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8372378 [details] [diff] [review]
Add feedback item to settings charm, using string from baseMenuOverlay.dtd

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Interim workaround for 963691
User impact if declined: No means to submit feedback without manually typing in to the url bar
Testing completed (on m-c, etc.): tested
Risk to taking this patch (and alternatives if risky): low risk, metro-only patch
String or IDL/UUID changes made by this patch:

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6bc693b11af0

Comment 15

[Kamil Jozwiak [:kjozwiak]]

Changing this back to "Resolved" and will complete the verification process once this lands in BETA and is tested using the test cases from comment #9

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/6bc693b11af0

Comment 17

[u279076]

Kamil, based on comment 14 this should be fixed in Firefox 28.0b7 which should be available Friday morning for you.

Comment 18

[Kamil Jozwiak [:kjozwiak]]

Went through the verification process using the following build:
- http://ftp.mozilla.org/pub/mozilla.org/firefox/releases/28.0b7/win32/en-US/

- Went through all the test cases from comment #9
- Went through the extra test cases from comment #12
- Went through the above test cases using both the X1 Carbon and the Surface Pro 2