MetroFX crash prompt: Allow positioning privacy policy link at arbitrary position in sentence

RESOLVED FIXED in Firefox 29

Status

Firefox for Metro
General
--
enhancement
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: TimAbraldes, Assigned: aryx)

Tracking

({verifyme})

unspecified
Firefox 29
x86
Windows 8.1
verifyme

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [mentor=tabraldes@mozilla.com][good first bug][lang=html] [feature] p=0)

Attachments

(1 attachment, 2 obsolete attachments)

+++ This bug was initially created as a clone of Bug #951802 +++

At the moment, the crashreporter has the following text:

<!ENTITY crashprompt.detailedMessage.thirdParagraph "We use crash reports to try to fix problems and improve our products. We handle your information as we describe in our ">
<!ENTITY crashprompt.privacyPolicyLink "&brandShortName; privacy policy">

This means the privacy policy has always to be at the end of the sentence which is problematic for localization.
We could do this by splitting the "crashprompt.detailedMessage.thirdParagraph" entity here [1] into "crashprompt.detailedMessage.thirdParagraphBeforeLink" and "crashprompt.detailedMessage.thirdParagraphAfterLink"

Then, in [2] we would replace "&crashprompt.detailedMessage.thirdParagraph;" with "&crashprompt.detailedMessage.thirdParagraphBeforeLink;" and add "&crashprompt.detailedMessage.thirdParagraphAfterLink;" after the link.

[1] https://mxr.mozilla.org/mozilla-central/source/browser/metro/locales/en-US/chrome/crashprompt.dtd?rev=b44cec692a98
[2] https://mxr.mozilla.org/mozilla-central/source/browser/metro/base/content/pages/crashprompt.xhtml?rev=b44cec692a98#103
Whiteboard: [mentor=tabraldes@mozilla.com][good first bug][lang=js]
Whiteboard: [mentor=tabraldes@mozilla.com][good first bug][lang=js] → [mentor=tabraldes@mozilla.com][good first bug][lang=html]

Updated

4 years ago
Whiteboard: [mentor=tabraldes@mozilla.com][good first bug][lang=html] → [mentor=tabraldes@mozilla.com][good first bug][lang=html] [feature] p=0
Created attachment 8356628 [details] [diff] [review]
patch, v1
Assignee: nobody → archaeopteryx
Status: NEW → ASSIGNED
Attachment #8356628 - Flags: review?(tabraldes)
Comment on attachment 8356628 [details] [diff] [review]
patch, v1

The patch in its current style would also cause trouble because inline content spans over several rows in the source code and linebreaks get converted to whitespaces. Removing the linebreaks and leading whitespaces would create a line of 223 characters. Should the content be generated by a script?
Attachment #8356628 - Flags: review?(tabraldes)
Flags: needinfo?(tabraldes)
I don't mind removing the linebreaks and having a long line, but needinfoing Matt in case he has a better suggestion
Flags: needinfo?(tabraldes) → needinfo?(mbrubeck)

Comment 5

4 years ago
You can just break the DTD entries into multiple lines.  See aboutTelemetry.dtd for examples.
In this case it's the XML content rather than the DTD where linebreaks are a problem.

I'm fine with one 223-char line here, or alternately I think you can put the whitespace into comment nodes, like:

  &before;<!--
  --><a>&link;</a><!--
  -->&after;
Flags: needinfo?(mbrubeck)
Created attachment 8356840 [details] [diff] [review]
patch, v2

Thanks for the suggestion, mbrubeck.
Attachment #8356628 - Attachment is obsolete: true
Attachment #8356840 - Flags: review?(tabraldes)
Comment on attachment 8356840 [details] [diff] [review]
patch, v2

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

Applied patch and verified that it works for me. Code looks good as well.

Can we have :flod take a look and make sure he likes this approach?
Attachment #8356840 - Flags: review?(tabraldes) → review+
Flags: needinfo?(francesco.lodolo)
My only suggestion would be to add a localization comment in the .dtd explaining that those 3 strings will be used together to create a single sentence.
Flags: needinfo?(francesco.lodolo)
(In reply to Francesco Lodolo [:flod] from comment #9)
> My only suggestion would be to add a localization comment in the .dtd
> explaining that those 3 strings will be used together to create a single
> sentence.

Sounds good!

:aryx - feel free to add the comment and carry forward my r+
https://hg.mozilla.org/integration/fx-team/rev/2fc2df126ccd
Keywords: checkin-needed
Whiteboard: [mentor=tabraldes@mozilla.com][good first bug][lang=html] [feature] p=0 → [mentor=tabraldes@mozilla.com][good first bug][lang=html] [feature] p=0[fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/2fc2df126ccd
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [mentor=tabraldes@mozilla.com][good first bug][lang=html] [feature] p=0[fixed-in-fx-team] → [mentor=tabraldes@mozilla.com][good first bug][lang=html] [feature] p=0
Target Milestone: --- → Firefox 29

Updated

4 years ago
No longer blocks: 861680

Updated

4 years ago
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.