1416875 - Rename prompt IDs from the form of info.body to infoBody

Status: RESOLVED FIXED

(Toolkit Graveyard :: Notifications and Alerts, enhancement, P5)

Description

[Jared Wein [:jaws] (please needinfo? me)]

Multiple places within our prompt code we have IDs for elements that use of the form of `info.body`. When referenced from CSS we have to escape the period and write the selector as `info\.body`. This makes it hard to search for the ID in our codebase as it appears in two different forms.

These are the three that need changing:
https://searchfox.org/mozilla-central/rev/bab833ebeef6b2202e71f81d225b968283521fd6/toolkit/components/prompts/content/commonDialog.xul#64,72,79

You will need to use searchfox.org to search the codebase for any references to `info.title`, `info\.title`, `info.icon`, `info\.icon`, `info.body`, and `info\.body`.

The same changes should be made to https://searchfox.org/mozilla-central/rev/bab833ebeef6b2202e71f81d225b968283521fd6/toolkit/components/prompts/content/tabprompts.xml#45-46

Comment 1

[Andrej Gorovoj]

Hi I would be happy to take this bug as my first one )

Comment 2

[Andrej Gorovoj]

Attachment: changes

not sure if I had to put any flags or what so ever, first attempt of me to do something in OSS and Firefox in general, hope to hear reviews

Comment 3

[Matthew N. [:MattN]]

Comment on attachment 8928143 [details] [diff] [review]
changes

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

Hello, in the future you should set the review flag. See https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction#Step_4_-_Get_your_code_reviewed

I think you forgot to update the CSS references that Jared mentioned though e.g. https://dxr.mozilla.org/mozilla-central/search?q=info%5C.body

Comment 4

[Andrej Gorovoj]

Attachment: newPatch

Silly mistake by me, did not figure out how mozilla's search engine works properly.. changed the rest of the code and added flag.

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8928448 [details] [diff] [review]
newPatch

Thanks, when you set the review flag, please set it to a '?' and then in the Requestee field put in ':jaws' (without the quotes).

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

Can you fold your patches together? You can use the `hg histedit` command to "roll" your patches together. See https://www.mercurial-scm.org/wiki/HisteditExtension for how to use the command.

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8928448 [details] [diff] [review]
newPatch

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

Thanks for writing these patches! Please see my note below and my previous note about rolling up the patches together.

::: toolkit/themes/osx/global/commonDialog.css
@@ +26,1 @@
>  #info\.header,

Please change this one too.

Comment 8

[Andrej Gorovoj]

Attachment: final?

Comment 9

[Andrej Gorovoj]

oh, it ended up being wrong format, I need to re-do it..

Comment 10

[Andrej Gorovoj]

Attachment: final maybe

hopefully this one contains everything needed.

Comment 11

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8929230 [details] [diff] [review]
final maybe

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

Thanks, this looks good. I'll push this patch to tryserver to make sure that all tests pass, then we can get this landed. In the meantime, can you add a commit message to this? You can use "Bug 1416875 - Renamed prompt IDs from the form of info.body to InfoBody so they won't need escaping. r=jaws". Then you can attach the commit with the commit message to this bug.

Comment 12

[Jared Wein [:jaws] (please needinfo? me)]

Link to trypush, https://treeherder.mozilla.org/#/jobs?repo=try&revision=143534dd6d0e928847155d28e86c2de4561820e1

Comment 13

[Andrej Gorovoj]

I've tryed to do that, but seems like you already did it?

Comment 14

[Jared Wein [:jaws] (please needinfo? me)]

I pushed the patch to our "try" server which will apply your patch and make sure that there are no test failures.

The patch still needs an updated commit message. If you need help setting the commit message you can join #introduction on the Mozilla IRC server at irc.mozilla.org.

Comment 15

[Jared Wein [:jaws] (please needinfo? me)]

Oh, I see what you mean, yes I did put a commit message on it when I pushed it to tryserver. I'll go ahead and land this for you now.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/491fbd1c506f
Renamed prompt IDs from the form of info.body to InfoBody so they won't need escaping. r=jaws

Comment 17

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/491fbd1c506f

Comment 18

[Jared Wein [:jaws] (please needinfo? me)]

Congratulations on landing your first patch! Thank you for your contribution to Firefox and Mozilla!

Comment 19

[Andrej Gorovoj]

Thank you so much for help :)