Closed Bug 957167 Opened 7 years ago Closed 7 years ago

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


(Firefox for Metro Graveyard :: General, enhancement)

Windows 8.1
Not set


(Not tracked)

Firefox 29


(Reporter: TimAbraldes, Assigned: aryx)



(Keywords: verifyme, Whiteboard: [][good first bug][lang=html] [feature] p=0)


(1 file, 2 obsolete files)

+++ 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.

Whiteboard: [][good first bug][lang=js]
Whiteboard: [][good first bug][lang=js] → [][good first bug][lang=html]
Whiteboard: [][good first bug][lang=html] → [][good first bug][lang=html] [feature] p=0
Attached patch patch, v1 (obsolete) — Splinter Review
Assignee: nobody → archaeopteryx
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)
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:

Flags: needinfo?(mbrubeck)
Attached patch patch, v2 (obsolete) — Splinter Review
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+
Keywords: checkin-needed
Whiteboard: [][good first bug][lang=html] [feature] p=0 → [][good first bug][lang=html] [feature] p=0[fixed-in-fx-team]
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [][good first bug][lang=html] [feature] p=0[fixed-in-fx-team] → [][good first bug][lang=html] [feature] p=0
Target Milestone: --- → Firefox 29
No longer blocks: metrobacklog
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.