484313 - DTD changes for bug 482874: Provide a friendlier/more useful alternative when the user encounters a 404 error page.

Status: RESOLVED INVALID

(Core :: Networking, defect)

Description

[Curtis Bartley [:cbartley]]

We need to add some new entity definitions for bug 482874: Provide a friendlier/more useful alternative when the user encounters a 404 error page.

Ideally, they'd just go into a single patch with all the changes for 482874.  Unfortunately the other changes are not ready yet, and string freeze is impending.  So the plan is to land a DTD only patch to get the strings in by the deadline.  

This patch will only add new ENTITY definitions, it will not change any existing ones so it should be very low risk.  The only real risk is leaving some code snarge if it turns out we don't need all the strings.  If this happens, then presumably we can remove unused strings without violating the string freeze.

Comment 1

[Curtis Bartley [:cbartley]]

Attachment: Hypothetical Patch (reference only)

Hypothetical patch for illustrative purposes only.  A real patch will be forthcoming.

Comment 2

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 368416 [details] [diff] [review]
Hypothetical Patch (reference only)

just a note. In case of full sentences, try no to chop them because it hardcodes structure.

Possible example in reference:
+<!ENTITY genericNotFoundError.goto "Go to">
+<!ENTITY genericNotFoundError.for "for">
+<!ENTITY genericNotFoundError.searchLabel "Or search Google:">

If this will be a sentence like:

Go to X for Y or search Z:

it's better to use JS and substitute variables with values.

Comment 3

[Curtis Bartley [:cbartley]]

(In reply to comment #2)
> (From update of attachment 368416 [details] [diff] [review])

> 
> it's better to use JS and substitute variables with values.

I was wondering about that.  I think I can do something like

   <!ENTITY genericNotFoundError.searchFor "Search [site] for [query]">
or <!ENTITY genericNotFoundError.searchFor "Search %s for %q">

Do we have an existing convention for this sort of thing?

Also, does it make sense to colons and quotes inside the entities? Do those ever get localized?

Comment 4

[Zibi Braniecki [:zbraniecki][:gandalf]]

> Do we have an existing convention for this sort of thing?

We usually keep JS l10n files in .properties, but if you have just one entity it may make sense to stick to DTD (Pike may say otherwise).
Regarding convention, we usually don't have variable substitution in DTD entities, so I don't think we have any strong convention here. I'd go with %s/%q or %1/%2. Don't forget to add a note describing variables

> Also, does it make sense to colons and quotes inside the entities? Do those
ever get localized?

Yes! Not all languages use those, not to mention RTL languages which will do their tricks.

Comment 5

[Axel Hecht [:Pike]]

I don't think we should take this for 1.9.1. 

Patches like this are prone to tweaks for some locales after landing, and this isn't a regression or anything as bad as to risk our string freeze.

Comment 6

[Curtis Bartley [:cbartley]]

Attachment: Entity definitions for new DNS Not Found and 404 Error pages.

Entity definitions for new DNS Not Found and 404 Error pages.

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 368469 [details] [diff] [review]
Entity definitions for new DNS Not Found and 404 Error pages.

>diff -r 6f21cfb3a12d browser/locales/en-US/chrome/overrides/netError.dtd
>--- a/browser/locales/en-US/chrome/overrides/netError.dtd	Tue Mar 17 18:16:55 2009 -0400
>+++ b/browser/locales/en-US/chrome/overrides/netError.dtd	Thu Mar 19 22:31:16 2009 -0700
>@@ -1,13 +1,33 @@
> <!ENTITY % brandDTD SYSTEM "chrome://branding/locale/brand.dtd">
> %brandDTD;
> 
> <!ENTITY loadError.label "Problem loading page">
> <!ENTITY retry.label "Try Again">
>+
>+<!-- genericNotFoundError error messages -->
>+
>+<!ENTITY genericNotFoundError.title404 "This page can't be found.">
>+<!ENTITY genericNotFoundError.titleDns "This address can't be found.">
>+<!ENTITY genericNotFoundError.suggestionLabel "Did you mean:">
>+<!ENTITY genericNotFoundError.anotherSuggestionLabel "or">
>+<!ENTITY genericNotFoundError.otherSuggestionsLabel "Other Suggestions:">
>+<!ENTITY genericNotFoundError.search "Search">
>+
>+<!-- "[site]" is a parameter and should not be localized. -->

nit: The proper format for a localization comment is:

<!-- LOCALIZATION NOTE (nameOFParameter): note here with bug reference if available  -->

nit2: %S should be a decent parameter name, shouldn't it?

Carry that on throughout and you have uir&r=beltzner for these. Hunt down mconnor for approval and checkin?

Comment 8

[Zibi Braniecki [:zbraniecki][:gandalf]]

> nit2: %S should be a decent parameter name, shouldn't it?

The problem with %S is that you cannot reverse order (first query, then site), so I suggest numbered variables like $1%s or %1.

Comment 9

[Curtis Bartley [:cbartley]]

Hey, I tried %s, %S, and %q, and it just doesn't work.  "%" seems to have a special meaning inside entity definitions.  Are we using this convention elsewhere?

Comment 10

[Curtis Bartley [:cbartley]]

OK, so it looks like the %s convention is the one used in property files, but it won't work in ENTITY definitions since % already has special meaning here.  It's been suggested that I really want to use properties here, but I'm under the impression that I need Chrome privileges to do that, and the about:notfounderror page that I'm working on (derived from the existing about:neterror page) is intentionally designed to drop privileges for security reasons.

Comment 11

[Curtis Bartley [:cbartley]]

Attachment: Entity definitions for new DNS Not Found and 404 Error pages Mk II

Comments now follow the "LOCALIZATION NOTE" form.  I retained the "[parameter]" syntax however.  I can't use the "%s" style used in property files since "%" has special meaning inside ENTITY definitions.  And I can't use properties since these definitions are for a page that runs without chrome privileges (cf about:neterror).  There's no real convention for parameterizing ENTITY definitions.  Johnath used the style "#1", "#2" etc. in "aboutCertError.dtd", but he had a simpler usage case than I've got.  Using the bracketed form offers some advantages for the implementation side.

Comment 12

[Curtis Bartley [:cbartley]]

mconnor requested some more detail about the parameter substitution I'm doing.  As mentioned in a previous comment, I can't use properties since I need to run in unprivileged HTML.  Gandalf's comments explain why parameterization is useful from a localization standpoint.  In this comment I'll just briefly explain how the parameterization works from the code side with some explanation as to why I did it that way.

In the DTD file I have:

  <!ENTITY genericNotFoundError.searchFor 'Search [site] for "[query]"'>

In the XHTML file I have

          <li id="searchFor">
            &genericNotFoundError.searchFor;
          </li>

Elsewhere in the XHTML file, I have the following JavaScript:

            expandParameters(searchForElem, {
                site: "news.bbc.co.uk", 
                query: queryElem
            });

This JavaScript does a text substitution for "[site]" and substitutes an HTML element for "[query]".  I won't go into detail about expandParameters() here, but I will note that it starts with the statement

          var fragments = elem.textContent.split(/[\[\]]/);

By using a bracketed representation, it becomes very easy to parse the parameterized representation and pull out the parameter names, I can just use the simple split statement.  Note that it doesn't matter what the bracket characters are, I could just as easily use "{site}" or even "#site#".  I chose the square brackets because I thought they were easiest to read.  Also I think the multi-character parameter names are easier to read (e.g. "query" instead of "q"), although for such small blocks of text it doesn't make much difference.

As a final note, I've intentionally designed the parameter substitution to allow us to insert HTML DOM elements, not just straight text.  Notably, the query in the example above is currently an anchor element.  This should simplify the localizers' jobs while giving us more flexibility on the layout and structure of the final XHTML file, which is unfortunately still in flux.

Comment 13

[Axel Hecht [:Pike]]

This clearly missed string freeze, I suggest to WONTFIX this one.

Comment 14

[Seth Bindernagel]

(In reply to comment #13)
> This clearly missed string freeze, I suggest to WONTFIX this one.

I agree with Axel.  We missed string freeze and should plan to fix after Firefox 3.5 Beta 4.  Should we resolve it as WONTFIX or leave it open?  I'm not sure WONTFIX is the best resolution.

Comment 15

[Mike Shaver (:shaver emeritus)]

WONTFIX means we wouldn't ever take it -- this seems like something that we can put on the trunk independent of a string freeze on the branch, right?

Comment 16

[Axel Hecht [:Pike]]

This bug is merely about pre-landing the strings for bug 482874. I don't think we should do that on trunk.

I don't think we should plan to take the 404 pages for 3.5 anymore.

That has of course no impact on when the actual bug fix lands on trunk.

Comment 17

[Mike Shaver (:shaver emeritus)]

Ah, OK.

Comment 18

[Mike Beltzner [:beltzner, not reading bugmail]]

This bug is INVALID or WONTFIX, I'll go with the slightly nicer option. Curtis, we should just attach a strings-only patch to bug 482874 instead and if it's felt that some or all of this is needed for Firefox 3.5 go through an exception process there.