1374811 - Page title not set in about:url-classifier

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, defect, P3)

Description

[François Marier [:francois]]

When I open about:url-classifier:

- the title of the window is just "Nightly", and 
- the title of the tab is the one from the page I was on before

Expected:

The title of both window and tab is "URL Classifier Information".

Comment 1

[Steve Pfister]

Figured this was as good of a first bug as any... 

Submitted a review and attached your name, but it says you're not accepting review requests.  Is there someone else I should choose as a reviewer?

Thanks!

Comment 2

[Dimi Lee [:dimi]]

(In reply to Steve Pfister from comment #1)
> Figured this was as good of a first bug as any... 
> 
> Submitted a review and attached your name, but it says you're not accepting
> review requests.  Is there someone else I should choose as a reviewer?
> 
> Thanks!

If you need a reviewer, you can add "r?dimi" in the end of patch description.
Thanks for help!

Comment 3

[Steve Pfister]

Attachment: Bug 1374811: Added title tag and changed the title of the page to URL Classifier Information;

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

Comment 4

[Steve Pfister]

Let me know if there's anything else I need to do.  Thanks!

Comment 5

[Dimi Lee [:dimi]]

Comment on attachment 8939708 [details]
Bug 1374811: Added title tag and changed the title of the page to URL Classifier Information;

https://reviewboard.mozilla.org/r/209562/#review216088

Looks good to me, thanks!

Comment 6

[Dimi Lee [:dimi]]

Hi Steve,
Please help change the bug description to r=dimi instead of r=francois, thanks.

Comment 7

[Steve Pfister]

Comment on attachment 8939708 [details]
Bug 1374811: Added title tag and changed the title of the page to URL Classifier Information;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/209562/diff/1-2/

Comment 8

[Steve Pfister]

Not sure if I did it the right way, but I ammended a new commit message and pushed for review again.  Is that what I was supposed to do in this instance?

Thanks!

Comment 9

[Steve Pfister]

Comment on attachment 8939708 [details]
Bug 1374811: Added title tag and changed the title of the page to URL Classifier Information;

https://reviewboard.mozilla.org/r/209562/#review216316

Comment 10

[Steve Pfister]

Also - what's next?  Do we just flag this bug as needing a check-in?  If I read the docs correctly, I think that's what is needed, but not 100% sure.

Comment 11

[Dimi Lee [:dimi]]

(In reply to Steve Pfister from comment #10)
> Also - what's next?  Do we just flag this bug as needing a check-in?  If I
> read the docs correctly, I think that's what is needed, but not 100% sure.

Yes, if you are ready to land this patch, flag checkin-needed and Sheriff will help land the patch.

Although this patch doesn't change much code, we should still run 'try' to make sure this patch doesn't break anything before landing.
See Try Wiki[1], you can also use Mozreview Automation Tab -> Trigger a Try Build.

If everything looks good then you can flag checkin-needed.

[1] https://wiki.mozilla.org/ReleaseEngineering/TryServer

Comment 12

[Steve Pfister]

I submitted a request for level 1 access.  Can you vouch for me?

Here's the link to the request:

https://bugzilla.mozilla.org/show_bug.cgi?id=1428767

Thanks!

Comment 13

[Steve Pfister]

Ok - I ran it through the try server... Lots of options, I see.  Picked the best ones and it all worked.

Comment 14

[Steve Pfister]

I'm not able to add the checkin-needed flag.  Can you guys add it for me?

Comment 15

[Dimi Lee [:dimi]]

Hi Steve,
Do you use
Edit Bug -> 'Tracking' category -> 'keywords' field? (The same field with good-first-bug)  

Add 'checkin-needed' to that field.

Comment 16

[Steve Pfister]

Sorry to keep this going... Looks like I don't have permission to change the keywords.  Can you do that?  And should I open a ticket w/ some team to get better permissions?

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/abb5be3dbdc7
Added title tag and changed the title of the page to URL Classifier Information; r=dimi

Comment 18

[Cristina Coroiu [:ccoroiu]]

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

Comment 19

[Francesco Lodolo [:flod]]

Hi, I'm really sorry to do this to a first patch, but I'm going to ask for a back-out. 

When a string changes, the string ID needs to change as well.
https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Changing_existing_strings

Comment 20

[Dorel Luca [:dluca]]

Backed out changeset abb5be3dbdc7 (bug 1374811) for changing existing strings without new IDs a=backout

https://hg.mozilla.org/mozilla-central/rev/0c2c46f76180a7d170faebdc84c21ea619f8d784

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

Comment 21

[Steve Pfister]

After reading the localization section, it says the key doesn't have to change if the purpose hasn't.  From what I see, the title is more clear and is being used in > 1 places.  I definitely could be wrong though. 

That said, do you guys have any suggestions on what to change the key to?

Comment 22

[Francesco Lodolo [:flod]]

(In reply to Steve Pfister from comment #21)
> After reading the localization section, it says the key doesn't have to
> change if the purpose hasn't.  

There is no mention of purpose in the section linked.

> If you are changing a string such that its meaning has changed, you must
> update the entity or property name for the string to match the new meaning. If
> your changes are relevant only for English — for example, to correct a
> typographical error or to make letter case consistent — then there is generally
> no need to update the entity name.

> That said, do you guys have any suggestions on what to change the key to?

aboutUrlClassifier.mainTitle or just aboutUrlClassifier.title would work, aboutUrlClassifier.pageTitle2 if you really want to keep the same name.

Comment 23

[Steve Pfister]

Comment on attachment 8939708 [details]
Bug 1374811: Added title tag and changed the title of the page to URL Classifier Information;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/209562/diff/2-3/

Comment 24

[Steve Pfister]

The key change was made and submitted for review.

Comment 25

[Francesco Lodolo [:flod]]

Comment on attachment 8939708 [details]
Bug 1374811: Added title tag and changed the title of the page to URL Classifier Information;

https://reviewboard.mozilla.org/r/209562/#review218426

Thanks, this looks good.

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f40f5ac940c1
Added title tag and changed the title of the page to URL Classifier Information; r=dimi,flod

Comment 27

[Stefan Hindli [:stefan_hindli]]

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