Closed Bug 903426 Opened 11 years ago Closed 11 years ago

[l10n] Change - In-content crash reporter prompt

Categories

(Firefox for Metro Graveyard :: Browser, defect, P2)

x86_64
Windows 8.1
defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 28

People

(Reporter: jimm, Assigned: TimAbraldes)

References

Details

(Whiteboard: [l10n] [block28] feature=defect c=feedback u=metro_firefox_user p=3)

Attachments

(4 files, 26 obsolete files)

161.30 KB, application/pdf
Details
156.48 KB, image/png
Details
96.12 KB, patch
TimAbraldes
: review+
Details | Diff | Splinter Review
28.82 KB, application/x-zip-compressed
Details
The original design of the crash prompt was signed off on by yuan and approved by the privacy group.  I'm not sure if both groups signed off on this new dialog approach since this new dialog thing is hideous.
Attached image screenshot (obsolete) —
Attached file original mockup (obsolete) —
Blocks: 831972, 892575
Attachment #788136 - Attachment description: Screenshot (63).png → screenshot
If we can't bring back the original styled ui easily in time for fx26, I'd suggest we make app modal dialogs a priority for the next train. Tim, curious what's preventing us from doing app modal dialogs like the original crash dialog we had?
Flags: needinfo?(tabraldes)
One thing we could look at if we can't do this up in content seamlessly is adding new winrt framework views. Not sure what's involved there but I think we have the ability to create multiple views/windows through winrt.
yet another option - embed the stylistic prompt in a new chrome tab. We had that going at one point, it would not be hard to switch back to it. We'd just need to tweak the design a bit. The good news, it could be designed using html.
Summary: New crash report submit prompt looks awful, needs privacy approval → Defect - New crash report submit prompt looks awful, needs privacy approval
Whiteboard: [preview-triage] feature=defect c=feedback u=metro_firefox_user p=0
We chatted in #windev, but I'll recap some of the discussion here so it's available more broadly.

I agree that the crash reporter prompt is hideous in its current state. It's a temporary solution; it was not signed off on and we need to come up with a permanent solution. I really like the idea of putting the crash reporter prompt in a tab. Other options are to make it a "notification" that hangs down from the tab strip like the "remember password" notification, or to make it a flyout. I prefer the tab idea, but we'll need UX to weigh in here.

Yuan: Does one of these approaches sound good or is there a better way to present the crash reporter prompt options?

If we decide that we really want the styled prompt to appear floating on top of content, we should extend the prompt service to make this possible on multiple platforms instead of making a metro-specific solution (e.g. create a Services.prompt.openCustomPrompt function), and prompts created this way should be tab-modal.


On app-modal dialogs:
  App-modal dialogs are undesirable: When we create an app-modal dialog, we are saying to the user that there is absolutely no way that he/she can continue interacting with the browser until he/she has responded to the dialog. There is no situation where this is actually true.
  On metro, we have a chance to break free from app-modal dialogs: We have other, better mechanisms for displaying information to the user (tab-modal prompts, notifications, flyouts, tabs) and none of our legacy code appears to rely on app-modal dialogs.
  The crash reporter prompt was the last app-modal dialog in metroFx. Converting it meant that we could rip app-modal prompts out of metroFx entirely. I didn't know what to convert it to at the time, so as a temporary solution I made it a prompt. Please forgive me for making the crash reporter prompt temporarily ugly! :)
Flags: needinfo?(tabraldes) → needinfo?(ywang)
I don't think we should make crash reporting dialog as tab modal dialogs. I'm sorry if I didn't make it clear before.

This is a general preference dialog. Unlike javascript alert, download requests, remember passwords, crash reporting dialog is not triggered by interactions from the web. So there is no direct relation in between the dialog and tabs. 

Also, Metro FX is not doing per crash reporting. The dialog is supposed to show up once only. It won't notify you again once you make a choice.  I think in this case it's legitimate to block the users interactions until they make a decision. 

Thoughts? Let me know if that makes sense to you.
Flags: needinfo?(ywang)
(In reply to Yuan Wang(:Yuan) – Firefox UX Team from comment #7)
> I don't think we should make crash reporting dialog as tab modal dialogs.
> I'm sorry if I didn't make it clear before.

We're all in agreement here: No one wants the crash reporter prompt to be a tab-modal prompt. I made it a tab-modal prompt temporarily when I was ripping out metroFx's app-modal dialog implementation, because it was the easiest/quickest thing to do at the time without breaking the crash reporter functionality.

> This is a general preference dialog. Unlike javascript alert, download
> requests, remember passwords, crash reporting dialog is not triggered by
> interactions from the web. So there is no direct relation in between the
> dialog and tabs. 
>
> Also, Metro FX is not doing per crash reporting. The dialog is supposed to
> show up once only. It won't notify you again once you make a choice.

This reminds me of the "would you like to disable sending performance information" notification that appears when you first run Nightly. That is a general preference that has no direct relation to the content of a particular tab. It is also supposed to show up once only, and not notify you again after you've made a choice.

That is implemented as a 'notification' and it does not block the user from using Nightly until he/she has made a choice. Since our crash reporter prompt has more text and we want it to look pretty, we could implement it as a kind of first-run page instead. I don't know exactly how we should present the option to the user, but I want to explore possibilities that do not block the user's interaction with the browser.

> I think in this case it's legitimate to block the users interactions
> until they make a decision. 

I sincerely disagree. Enabling the crash reporter is just a preference, and from the user's perspective it's not even a very important one. I don't think that we should block the user from interacting with the browser until he/she has made a decision about it. Some users might even want to navigate to some Mozilla web pages to help them decide whether to enable the feature.
(In reply to Tim Abraldes [:TimAbraldes] [:tabraldes] from comment #8)
> (In reply to Yuan Wang(:Yuan) – Firefox UX Team from comment #7)
> > I don't think we should make crash reporting dialog as tab modal dialogs.
> > I'm sorry if I didn't make it clear before.
> 
> We're all in agreement here: No one wants the crash reporter prompt to be a
> tab-modal prompt. I made it a tab-modal prompt temporarily when I was
> ripping out metroFx's app-modal dialog implementation, because it was the
> easiest/quickest thing to do at the time without breaking the crash reporter
> functionality.
> 
> > This is a general preference dialog. Unlike javascript alert, download
> > requests, remember passwords, crash reporting dialog is not triggered by
> > interactions from the web. So there is no direct relation in between the
> > dialog and tabs. 
> >
> > Also, Metro FX is not doing per crash reporting. The dialog is supposed to
> > show up once only. It won't notify you again once you make a choice.
> 
> This reminds me of the "would you like to disable sending performance
> information" notification that appears when you first run Nightly. That is a
> general preference that has no direct relation to the content of a
> particular tab. It is also supposed to show up once only, and not notify you
> again after you've made a choice.
> 
> That is implemented as a 'notification' and it does not block the user from
> using Nightly until he/she has made a choice. Since our crash reporter
> prompt has more text and we want it to look pretty, we could implement it as
> a kind of first-run page instead. I don't know exactly how we should present
> the option to the user, but I want to explore possibilities that do not
> block the user's interaction with the browser.
> 
> > I think in this case it's legitimate to block the users interactions
> > until they make a decision. 
> 
> I sincerely disagree. Enabling the crash reporter is just a preference, and
> from the user's perspective it's not even a very important one. I don't
> think that we should block the user from interacting with the browser until
> he/she has made a decision about it. Some users might even want to navigate
> to some Mozilla web pages to help them decide whether to enable the feature.

I like the idea of integrating crash reporting dialog with first-run experience. I am right now working on that.

Presenting the dialog in first run feels less intrusive. The down side is that the decision-making is out of context, and the users can skip the first-run if they want. So we still need to show a message dialog after crashes if the users chose to skip first run. But that group should be a lot smaller.

In terms of whether to block the interactions, neither of us actually have any data about crash reporting. I imagine different demographics might have opposite preferences on whether they care about crash reporting or not. If we want to allow users' interactions while not dismiss the dialog, side flyout might be a better choice. But I would like to try a few ideas and talk to UX team and privacy team next week.

Will keep you posted.
Thanks for looking into this, Yuan!

In desktop Firefox, we show a "oops you crashed" page on startup if we detect that the previous session crashed. Maybe we could have a similar page in metro Firefox that asks the user whether they want to enable crash reporting? This approach would let us re-use a lot of the work in bug 886794, making it quick to implement.
Blocks: metrov1backlog
No longer blocks: metrov1defect&change
Attached file Crash report_0812.pdf (obsolete) —
Tim, I was exploring different ways of showing the dialog, including using splash screen, flyout, or info bar. It's difficult to indicate that the message is coming from the app not the webpage.

And I think maybe we could take another approach here. Instead of showing the message after restoring FX, we take the same approach as Android and desktop FX, show the message dialog right after the first time crash occurs.

In this case, it's okay to use app-modal dialog as there is no other interactions with the browser other than restart or close. And we can use a more consistent language here, which I think privacy team will be quite happy about. 

Let me know what you think. I am going to get input from Alina about this idea as well
> And I think maybe we could take another approach here. Instead of showing
> the message after restoring FX, we take the same approach as Android and
> desktop FX, show the message dialog right after the first time crash occurs.

Unfortunately we can't do this in metro. We have no way of getting dialog up on top of the metro interface. Once the browser crashes, it's gone. Prompting has to wait until the browser restarts. Hence the interest in using an about tab.

I think app modal prompts need to come back eventually, but I wonder if we can work out app modal dialog issues before the uplift (Tim?). In the mean time we need to find a themed solution to our existing tab prompts, and a suitable quasi app modal fix for prompts like the crash prompt. Hence the interest in using an about page.
Blocks: 887176
Thanks for the info Jim. I am working on making an in-content page for crash reporting dialog. Will post them later today.
(In reply to Jim Mathies [:jimm] from comment #12)
[...] 
> I think app modal prompts need to come back eventually, but I wonder if we
> can work out app modal dialog issues before the uplift (Tim?).

I am still opposed to implementing app modal dialogs in metro: The crash reporter prompt is the only potential consumer of app modal dialogs, and I believe that we have better alternatives for the crash reporter prompt (in particular the about page idea).

If we do feel that we have a consumer of app modal dialogs that absolutely can't be made in a non-app-modal way, then we'd probably have to create a metro-specific copy of toolkit/prompts/src/nsPrompter.js that handles both tab-modal and app-modal dialogs. This would be a shame since it would mean that metro and desktop no longer share a common prompts implementation. I'm not sure if this could be implemented in time for v1.

> In the mean
> time we need to find a themed solution to our existing tab prompts,

Indeed! Bug 801187.

> and a
> suitable quasi app modal fix for prompts like the crash prompt. Hence the
> interest in using an about page.

Yes, but I would call the about page the "better option" rather than "a suitable quasi app modal fix" :)
Summary: Defect - New crash report submit prompt looks awful, needs privacy approval → [MP] Defect - New crash report submit prompt looks awful, needs privacy approval
Whiteboard: [preview-triage] feature=defect c=feedback u=metro_firefox_user p=0 → [preview] feature=defect c=feedback u=metro_firefox_user p=0
Attached file Crash report v3_0813.pdf (obsolete) —
I talked with Michael Maslaney in Shorlander's team about his design on in-content page on Firefox desktop. 
I think we should apply a consistent visual language here. I did not use gradient on the rounded buttons or the background as he did for desktop. 

But I think it is the fastest way to get our stuff out the door. 

We will need new fonts, Feura Sans here. I will send them to Tim via email. 

In the mean time, I will post this and ask for privacy review. Let me know if there is any question. I can send out production or specs later if needed :)
Attachment #789340 - Attachment is obsolete: true
Looks great! I'll check this out in-depth a little more tomorrow and see if I can create a WIP patch
Attached file Crash report_0813.pdf (obsolete) —
Slightly change on the typography and font color to make it consistent with desktop pages.

Will sync up with Alina on Thursday afternoon. Hopefully there won't be any major change.
Attachment #789929 - Attachment is obsolete: true
Attached file Crash report_0816.pdf (obsolete) —
After a review with Alina today, she gave a pass on the in-content approach we are taking. There are a few changes on the design:

1. Alina will provide a short description of crash reporting feature for us to show on the in-content page. She will also help us finalize the wording of the links.

2. From the privacy perspective, opening the privacy link in a new tab is not helping the users come back and make a choice. So the privacy team suggested to use the lightbox pop-up to display the entire policy statement, so that the user will always stay on the in-content page. 

A question for Tim, if we are going to surface the content from http://www.mozilla.org/en-US/legal/privacy/firefox.html. Is the content going to be hard-coded or not? 

Please find the attached document for details.
Attachment #789946 - Attachment is obsolete: true
Did we ever get the short description & the finalized wording of the links from Alina?
Whiteboard: [preview] feature=defect c=feedback u=metro_firefox_user p=0 → [preview][blocked] feature=defect c=feedback u=metro_firefox_user p=0
Whiteboard: [preview][blocked] feature=defect c=feedback u=metro_firefox_user p=0 → [preview][blocked][l10n] feature=defect c=feedback u=metro_firefox_user p=0
Whiteboard: [preview][blocked][l10n] feature=defect c=feedback u=metro_firefox_user p=0 → [l10n] [preview][blocked][l10n] feature=defect c=feedback u=metro_firefox_user p=0
Attached patch Patch v1 (WIP) (obsolete) — Splinter Review
This patch gets us part of the way there.

Still to do:
  - Lightbox with privacy policy
  - Correctly hook up an "about:" URI to point to the page
  - Make startup crash check open the about page
Assignee: nobody → tabraldes
Status: NEW → ASSIGNED
Attached image Patch v1 Screenshot (obsolete) —
This is a screenshot of what the in-content prompt looks like with the patch applied
Hey Tim, can you provide a point estimate for this defect.
Blocks: metrov1it14
No longer blocks: metrov1backlog
Flags: needinfo?(tabraldes)
Priority: -- → P2
QA Contact: jbecerra
Whiteboard: [l10n] [preview][blocked][l10n] feature=defect c=feedback u=metro_firefox_user p=0 → [l10n] [preview][blocked] feature=defect c=feedback u=metro_firefox_user p=0
(In reply to Yuan Wang(:Yuan) – Firefox UX Team from comment #18)
[...]
> A question for Tim, if we are going to surface the content from
> http://www.mozilla.org/en-US/legal/privacy/firefox.html. Is the content
> going to be hard-coded or not? 

I'm not sure how to create a lightbox that has content from another page. One of the front-end engineers might know if this is possible. One consideration that complicates matters is that we would have to grab correctly localized content, not just English-US.

Yuan I have a couple of questions:
  1. I noticed that the button colors in the design (blue for default, dark grey for non-default) are different from the button colors we've been using in metroFx (orange for default, light grey for non-default). Do we want to stick with the metroFx colors or use the colors in the new design? Either way is easy to do, I'm just curious.
  2. Do we have updated strings from Alina to use?
  3. I couldn't find arrow assets that looks like the ones in the design. Do we need new assets for the blue arrow? In the current patch I'm using a placeholder asset but it doesn't look very good.
  4. I'm not totally sure what to do with the font that you sent me for this patch. Could we chat via IRC sometime this week to go over what I should with the font?
Flags: needinfo?(ywang)
(In reply to Marco Mucci [:MarcoM] from comment #22)
> Hey Tim, can you provide a point estimate for this defect.

p=3
Flags: needinfo?(tabraldes)
Whiteboard: [l10n] [preview][blocked] feature=defect c=feedback u=metro_firefox_user p=0 → [l10n] [preview][blocked] feature=defect c=feedback u=metro_firefox_user p=3
Summary: [MP] Defect - New crash report submit prompt looks awful, needs privacy approval → [MP] Change - In-content crash reporter prompt
Attached patch Patch v2 (WIP) (obsolete) — Splinter Review
This patch adds the lightbox with the privacy policy. I noticed that the second paragraph of the privacy policy is the same string as what appears when you expand the 'what does a crash report include?' link. We'll want to make sure we have different/updated strings for these.

Still to do:
  - Correctly hook up an "about:" URL to the crash prompt page
  - Make the browsers startup check show the about page
  - Fix issues with lightbox when the page is scrolled
Attachment #796441 - Attachment is obsolete: true
Attached image Patch v2 screenshot (obsolete) —
Attachment #796442 - Attachment is obsolete: true
I asked Alina about the localization for private policy. She said currently there is no localized version. She mentioned that process requires lots of legal approval. So I think we will have to display English content here.


(In reply to Tim Abraldes [:TimAbraldes] [:tabraldes] from comment #23)
> (In reply to Yuan Wang(:Yuan) – Firefox UX Team from comment #18)
> [...]
> > A question for Tim, if we are going to surface the content from
> > http://www.mozilla.org/en-US/legal/privacy/firefox.html. Is the content
> > going to be hard-coded or not? 
> 
> I'm not sure how to create a lightbox that has content from another page.
> One of the front-end engineers might know if this is possible. One
> consideration that complicates matters is that we would have to grab
> correctly localized content, not just English-US.
> 
> Yuan I have a couple of questions:
>   1. I noticed that the button colors in the design (blue for default, dark
> grey for non-default) are different from the button colors we've been using
> in metroFx (orange for default, light grey for non-default). Do we want to
> stick with the metroFx colors or use the colors in the new design? Either
> way is easy to do, I'm just curious.

Good question. We have been using orange for default on our message dialogs and info bars. On Firefox desktop, this in-content page has default in blue. So I kept the original blue color there. I don't have a strong preference here. Let's use the orange color to make it the same with other dialogs.

>   2. Do we have updated strings from Alina to use?
I have not heard from her about the updated strings yet. I will ping her today.

>   3. I couldn't find arrow assets that looks like the ones in the design. Do
> we need new assets for the blue arrow? In the current patch I'm using a
> placeholder asset but it doesn't look very good.

Okay. I will create them for you.

>   4. I'm not totally sure what to do with the font that you sent me for this
> patch. Could we chat via IRC sometime this week to go over what I should
> with the font?

Okay. I will be back after Labor day weekend, we can chat on Sept 3rd. Basically I wanted to keep the same visual style (font, color choice) as the upcoming design on FX desktop. However, desktop has not implemented those visuals yet. So if it end up being too tricky to do, I'm okay going with the default font we've been using on Metro.
Flags: needinfo?(ywang)
Attached patch Patch v3 (obsolete) — Splinter Review
OK I think all the issues are worked out. The buttons are hooked up and everything seems to be working correctly.

This patch cleans up the logic surrounding the crash reporter prefs and also when crash reports are sent.

There's some additional investigation/cleanup for me to do, so I'm not requesting review quite yet.
Attachment #797629 - Attachment is obsolete: true
Attached patch Patch v4 (obsolete) — Splinter Review
This patch has all the code that I expect to go into the final landing.

It does not include:
  - The correct "arrow" images for expanding/collapsing "What's in a crash report?"
  - Correct/finalized strings. Most notably, the "What's in a crash report" text is the same as the second paragraph of the "Learn more about Nightly privacy policy" lightbox
Attachment #798119 - Attachment is obsolete: true
Attachment #799081 - Flags: review?(mbrubeck)
Attached patch Patch v5 (obsolete) — Splinter Review
Fixing a couple issues with the last patch
Attachment #799081 - Attachment is obsolete: true
Attachment #799081 - Flags: review?(mbrubeck)
Attachment #799094 - Flags: review?(mbrubeck)
Comment on attachment 799094 [details] [diff] [review]
Patch v5

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

Not sure what the workflow here is (if it's just a preliminary review), but please don't ever land "temporary" strings on mozilla-central. 
It's clear the two last paragraph are a pain to localize, and the last one looks completely out of place (references to Persona, Firefox 3.0.x, etc.)

::: browser/metro/locales/en-US/chrome/crashprompt.dtd
@@ +3,5 @@
> +   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> +
> +
> +<!ENTITY crashprompt.title           "Would you like to send Mozilla crash reports?">
> +<!ENTITY crashprompt.message         "We are sorry, Nightly just recovered from a crash. Sending crash reports will help Mozilla make Nightly more stable and secure. You can always change your preference in Settings/Options.">

Hard-coded "Nightly" seems very wrong here.
(In reply to Francesco Lodolo [:flod] from comment #31)
> Not sure what the workflow here is (if it's just a preliminary review), but
> please don't ever land "temporary" strings on mozilla-central. 
> It's clear the two last paragraph are a pain to localize, and the last one
> looks completely out of place (references to Persona, Firefox 3.0.x, etc.)

I was hoping to get review on just the code pieces, and hold the patch until the strings are finalized. I wasn't planning to land with the existing strings.
 
> ::: browser/metro/locales/en-US/chrome/crashprompt.dtd
> @@ +3,5 @@
> > +   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> > +
> > +
> > +<!ENTITY crashprompt.title           "Would you like to send Mozilla crash reports?">
> > +<!ENTITY crashprompt.message         "We are sorry, Nightly just recovered from a crash. Sending crash reports will help Mozilla make Nightly more stable and secure. You can always change your preference in Settings/Options.">
> 
> Hard-coded "Nightly" seems very wrong here.

I'll fix this in the next version of the patch
Weren't those long strings (the privacy policy) already approved by the privacy group for the previous implementation of the crash prompt? (bug 886794)

Also there's no need to use release channel in the long strings if that makes it easier for l10n.
(In reply to Jim Mathies [:jimm] from comment #33)
> Also there's no need to use release channel in the long strings if that
> makes it easier for l10n.

Not sure what you mean. 

Since I saw the review request, I was just asking to not land strings that are not final (Tim answered for that), and the final one (crashprompt.privacyPolicy.secondParagraph) still looks quite odd to me.
(In reply to Francesco Lodolo [:flod] from comment #34)
> (In reply to Jim Mathies [:jimm] from comment #33)
> > Also there's no need to use release channel in the long strings if that
> > makes it easier for l10n.
> 
> Not sure what you mean. 

Crash reporting defaults to autosubmit on central and aurora and these strings are specific to the browser, so we don't need to escape brand strings like &brandShortName; if don't want to, we can hard code "Firefox" or "Mozilla" if need be.

> 
> Since I saw the review request, I was just asking to not land strings that
> are not final (Tim answered for that), and the final one
> (crashprompt.privacyPolicy.secondParagraph) still looks quite odd to me.

These were approved by privacy for use, and landed previously in bug 886794. See bug 885129 and https://bug876031.bugzilla.mozilla.org/attachment.cgi?id=761188.
Attached patch Part 1 patch v6 (obsolete) — Splinter Review
Updated button styles and incorporated the new arrowbox assets
Attachment #799094 - Attachment is obsolete: true
Attachment #799094 - Flags: review?(mbrubeck)
Attachment #800432 - Flags: review?(mbrubeck)
Attached image Patch v6 sreenshot (obsolete) —
Attachment #797630 - Attachment is obsolete: true
Blocks: metrov1it15
No longer blocks: metrov1it14
Attachment #800432 - Flags: review?(mbrubeck) → review+
Is this ready to land?
The finalized strings are not yet available so this isn't able to land.
Blocks: metrov1it16
No longer blocks: MetroPreviewRelease, metrov1it15
Summary: [MP] Change - In-content crash reporter prompt → Change - In-content crash reporter prompt
Whiteboard: [l10n] [preview][blocked] feature=defect c=feedback u=metro_firefox_user p=3 → [l10n] [blocked] feature=defect c=feedback u=metro_firefox_user p=3
Whiteboard: [l10n] [blocked] feature=defect c=feedback u=metro_firefox_user p=3 → [l10n] [blocked] [preview] feature=defect c=feedback u=metro_firefox_user p=3
Blocks: metrov1it17
No longer blocks: metrov1it16
No longer blocks: 892575
Whiteboard: [l10n] [blocked] [preview] feature=defect c=feedback u=metro_firefox_user p=3 → [l10n] [blocked] feature=defect c=feedback u=metro_firefox_user p=3
Attached file Crash report_1014.pdf (obsolete) —
Alina mentioned privacy and legal would like to see a consistent approach on crash reporting prompt. Currently Metro is sending URLs automatically, which could cause potential privacy concerns. 
She suggested us to provide an option for users to opt in or opt out.
Please see updated mockup.
Attachment #791499 - Attachment is obsolete: true
(In reply to Yuan Wang(:Yuan) – Firefox UX Team from comment #40)
> Created attachment 816873 [details]
> Crash report_1014.pdf
> 
> Alina mentioned privacy and legal would like to see a consistent approach on
> crash reporting prompt.

Is "would like" the same as "blocks Metro from shipping a crash reporter"? I don't think we should be making any further investment in this for v1 unless it's mandated by legal.
(In reply to Asa Dotzler [:asa] from comment #41)
> (In reply to Yuan Wang(:Yuan) – Firefox UX Team from comment #40)
> > Created attachment 816873 [details]
> > Crash report_1014.pdf
> > 
> > Alina mentioned privacy and legal would like to see a consistent approach on
> > crash reporting prompt.
> 
> Is "would like" the same as "blocks Metro from shipping a crash reporter"? I
> don't think we should be making any further investment in this for v1 unless
> it's mandated by legal.

Alina, could you clarify Asa's question? Is this option "include URL address" required to have for v1 release by legal?
Flags: needinfo?(ahua)
I talked with Alina this week about landing this with temporary strings and file a separate follow-up bug. She thought it should be okay to do so.

And she confirmed that it's required by legal to have the option of sending with or without URLs.
Flags: needinfo?(ahua)
We can't land temporary strings; that burdens our localizers. I'll start working on the new version of this, but we'll still need finalized strings at some point.

I noticed that the mock-up doesn't include a way for users to change whether we send URLs from the options flyout. Yuan - I'm guessing we should have that?
Flags: needinfo?(ywang)
(In reply to Tim Abraldes [:TimAbraldes] [:tabraldes] from comment #44)
> We can't land temporary strings; that burdens our localizers. I'll start
> working on the new version of this, but we'll still need finalized strings
> at some point.
> 
> I noticed that the mock-up doesn't include a way for users to change whether
> we send URLs from the options flyout. Yuan - I'm guessing we should have
> that?

Yes, you are right. We need to have an option to change that. 

I will follow up with Alina on getting strings finalized.
Flags: needinfo?(ywang)
Whiteboard: [l10n] [blocked] feature=defect c=feedback u=metro_firefox_user p=3 → [l10n] [block28] feature=defect c=feedback u=metro_firefox_user p=3
Blocks: metrov1it18
No longer blocks: metrov1it17
Attached patch Part 2 patch v1 (obsolete) — Splinter Review
This patch adds an "include URLs" checkbox to the crash reporter prompt page, and adds an option for including URLs in the options flyout under the crash reporter heading.

Disabling crash reporting automatically disables including URLs and enabling sending of URLs automatically enables crash reporting.

I haven't tested that the URLs actually get stripped out of the crash report, mostly because I haven't been able to figure out where to find URLs that are included in a crash report. Everything else seems to work pretty well.
Attachment #827541 - Flags: review?(mbrubeck)
Attachment #800432 - Attachment description: Patch v6 → Part 1 patch v6
Attached image Screenshot with patches applied (obsolete) —
This screenshot shows the crash reporter prompt page with the new checkbox, and shows the new option in the prefs flyout.
Attachment #800439 - Attachment is obsolete: true
Attachment #827542 - Attachment is patch: false
Attachment #827542 - Attachment mime type: text/plain → image/png
Attachment #788141 - Attachment is obsolete: true
Comment on attachment 827541 [details] [diff] [review]
Part 2 patch v1

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

Looks good; just a couple of minor questions:

::: browser/metro/base/content/pages/crashprompt.xhtml
@@ +101,5 @@
>        </p>
> +      <div id="crashpromptURLCheckboxContainer">
> +        <label id="crashpromptIncludeURLLabel">
> +          <input type="checkbox"
> +                 id="crashpromptIncludeURLCheckbox" />

When this page loads, should we should initialize the checkbox based on the current preference state?

::: browser/metro/locales/en-US/chrome/preferences.dtd
@@ +34,5 @@
>  <!ENTITY doNotTrack.learnMoreLink                                "Learn more&#x2026;">
>  
>  <!ENTITY optionsHeader.reporting.title                           "Crash Reporter">
>  <!ENTITY optionsHeader.reporting.crashes.label                   "&brandShortName; submits crash reports to help Mozilla make your browser more stable and secure">
> +<!ENTITY optionsHeader.reporting.crashes.submitURLs             "Include the URL of the page I was on">

Should we s/URL/address/ to match the other string?
Attachment #827541 - Flags: review?(mbrubeck) → review-
Attached patch Part 2 patch v2 (obsolete) — Splinter Review
(In reply to Matt Brubeck (:mbrubeck) from comment #48)
> Comment on attachment 827541 [details] [diff] [review]
> Part 2 patch v1
> 
> Review of attachment 827541 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good; just a couple of minor questions:
> 
> ::: browser/metro/base/content/pages/crashprompt.xhtml
> @@ +101,5 @@
> >        </p>
> > +      <div id="crashpromptURLCheckboxContainer">
> > +        <label id="crashpromptIncludeURLLabel">
> > +          <input type="checkbox"
> > +                 id="crashpromptIncludeURLCheckbox" />
> 
> When this page loads, should we should initialize the checkbox based on the
> current preference state?

Sure, done.

> ::: browser/metro/locales/en-US/chrome/preferences.dtd
> @@ +34,5 @@
> >  <!ENTITY doNotTrack.learnMoreLink                                "Learn more&#x2026;">
> >  
> >  <!ENTITY optionsHeader.reporting.title                           "Crash Reporter">
> >  <!ENTITY optionsHeader.reporting.crashes.label                   "&brandShortName; submits crash reports to help Mozilla make your browser more stable and secure">
> > +<!ENTITY optionsHeader.reporting.crashes.submitURLs             "Include the URL of the page I was on">
> 
> Should we s/URL/address/ to match the other string?

This string, along with all of the other strings in this patch, is not finalized. I've updated the string based on this suggestion, but it needs signoff from UX and privacy/legal.
Attachment #827541 - Attachment is obsolete: true
Attachment #827676 - Flags: review?(mbrubeck)
Attachment #827676 - Flags: review?(mbrubeck) → review+
Part 1, crashprompt.dtd: is there a specific reason for mixing hard-coded Nightly and &brandShortName;?
Attached patch Part 1 patch v7 (obsolete) — Splinter Review
(In reply to Francesco Lodolo [:flod] from comment #50)
> Part 1, crashprompt.dtd: is there a specific reason for mixing hard-coded
> Nightly and &brandShortName;?

No reason that I recall; they should probably all be &brandShortName;

Updated patch, carrying forward r+

Also, everyone please recall that the strings in these patches will be changing when we get finalized versions from the privacy team
Attachment #800432 - Attachment is obsolete: true
Attachment #828137 - Flags: review+
I checked in with Alina on Monday. She is working on the strings. I told her that the team needs to have strings by the last iteration before uplift(the week of Nov 25). I will check in with her next week again.
Patch is now r+ - waiting on strings before landing.
Blocks: metrov1backlog
No longer blocks: metrov1it18
Hi Alina, want to confirm the strings will be ready at the beginning of next week as it will be our last development iteration before our uplift to Aurora 28?
Flags: needinfo?(ahua)
I'm meeting with Jishnu on this today and we have a meeting with Yuan and Karen on Friday. Yuan or I will provide an update as soon as possible.
Flags: needinfo?(ahua)
Summary: Change - In-content crash reporter prompt → [l10n] Change - In-content crash reporter prompt
Attached patch Part 2 patch v3 (obsolete) — Splinter Review
I finally tested this patch and discovered that it wasn't actually stripping URLs. Updated so that now it does.
Attachment #827676 - Attachment is obsolete: true
Attachment #8336477 - Flags: review+
Blocks: metrov1it20
No longer blocks: metrov1backlog
I responded with specifics combining information from Tim and Laura in the actual privacy bug. 

Alina - I believe that the in-crash dialogue is something that legal will *not* localize, and something that our l10n community will need to localize. That means we need this text ASAP in time for the uplift to Aurora on Dec 9 (the cut-off for string changes).

Can we have the final text this week to give Tim time to update the text in time for uplift?

Possible?
Thanks, Karen
Flags: needinfo?(ahua)
Hi Karen - I don't see your update in the privacy bug (bug 885129). Can you confirm which bug you commented in?  Could you be sure to see the updates I added to the privacy review bug after our last call? 

One clarification: The final text is dependent on Laura vetting and/or updating the description for 'what is a crash report?'. If this is done, Jishnu and I will need to review it and can probably get back to you pretty quickly. Also, will the UX for this be updated for us to review as well?  

Thanks!
Flags: needinfo?(ahua)
well I'm scuppered. I had a huge long response in 885129 and you're right - it's not there!! (scuppered is not the word I would use, but this is a public forum). 

Here's a summary.

- The 'what is a crash report' statement is, on the whole, pretty encompassing of the type of data we could be collecting
- There is some browser data (like the version, whether you were using the desktop or metro theme, which release channel was used, what add-ons are installed etc) that we could also collect, therefore if we want to be fully comprehensive, we could say 
" ... some details about the crash, your browser and your device... "
- Minor note to ensure that the word 'device' seems ok when we're talking about the hardware, which could be tablet, laptop etc. Otherwise we may want to use a more generic 'hardware', as I personally think of a 'device' in the sense of a smartphone/tablet. But happy to use industry lingo here and what desktop uses here, too.
- Minor note that we state 'open pages', which should be the same thing as 'active tabs', provided you think that the users would understand that it could be any open tab that we collect the URL from (the one we suspect caused the crash)

There is a host of information (covered by the language used) that *could* be collected, but only the best-guess culprits are actually sent to Mozilla.
Updated the strings and add a checkbox option of include URL address in Options.
Attachment #816873 - Attachment is obsolete: true
I think the next steps for this bug are as follows:
  1. Update the patch to incorporate any/all recent string changes (this includes copy+pasting the entire contents of the privacy policy into the lightbox).
  2. Land the patch in this bug.
  3. File a follow-up bug: The lightbox contents should be loaded dynamically from the privacy policy page, whose URL is available through this pref [1].
  4. File a follow-up bug: "Include URLs" should be a checkbox, not a toggle

The reason for leaving 3 and 4 as follow-ups is that they will take some amount of time to implement and will require review, but they do not affect strings so they can be fixed at a later time than the rest of this bug.

I've unbitrotted the patch in this bug locally, and am building/testing with the new string changes, with the hope of landing tomorrow (Wednesday, December 5th)

[1] https://mxr.mozilla.org/mozilla-central/source/browser/app/profile/firefox.js?rev=dcfc0e0e663d#952
(I'm doing a lot of assumptions, not sure if I'm understanding it right)

>   1. Update the patch to incorporate any/all recent string changes (this
> includes copy+pasting the entire contents of the privacy policy into the
> lightbox).

How much text are we talking about? Because including privacy policies, even more as a temporary solution, sounds like a bad idea.

Also: privacy policies have legal value, is legal OK with exposing a privacy policy localized in this way?

>   3. File a follow-up bug: The lightbox contents should be loaded
> dynamically from the privacy policy page, whose URL is available through
> this pref [1].

Well, I would much prefer to fix this. If strings aren't involved as I think, you're free to fix it in Aurora instead of central.

> The reason for leaving 3 and 4 as follow-ups is that they will take some
> amount of time to implement and will require review, but they do not affect
> strings so they can be fixed at a later time than the rest of this bug.

If they render the "privacy policy" localized at point 1 unnecessary, they kind of have impact on strings (we're asking people to localize a bunch of legal text while we fix 3).
(In reply to Tim Abraldes [:TimAbraldes] [:tabraldes] from comment #61)
> I think the next steps for this bug are as follows:
>   1. Update the patch to incorporate any/all recent string changes (this
> includes copy+pasting the entire contents of the privacy policy into the
> lightbox).

Lets just leave the 'lightbox' blank and file a follow up that blocks beta28 on getting remote privacy policy content loading into it. There's no point in putting l10n folks through the translation headache if we don't intend on using the strings they translate for release.

If for some reason we can't get the iframe loading to work, we'll skip displaying it in-app and just link to the privacy page somewhere.
http://www.mozilla.org/en-US/legal/privacy/firefox.html

We are going to need an online version of this page that matches our styling as well. Lets get that filed too.
(In reply to Jim Mathies [:jimm] from comment #63)
> (In reply to Tim Abraldes [:TimAbraldes] [:tabraldes] from comment #61)
> > I think the next steps for this bug are as follows:
> >   1. Update the patch to incorporate any/all recent string changes (this
> > includes copy+pasting the entire contents of the privacy policy into the
> > lightbox).
> 
> Lets just leave the 'lightbox' blank and file a follow up that blocks beta28
> on getting remote privacy policy content loading into it. There's no point
> in putting l10n folks through the translation headache if we don't intend on
> using the strings they translate for release.
> 
> If for some reason we can't get the iframe loading to work, we'll skip
> displaying it in-app and just link to the privacy page somewhere.

I just want to note the reason we chose to display it in a lightbox instead of online. 

It's intended to direct the users back to the prompt after in-depth reading on the policy content. In comparison, a link to the actual website in a new tab, will direct the users away. Given the fact that content is full-screen on metro, it's highly possible that the users will not come back to choose their preference once they are directed away.
(In reply to Yuan Wang(:Yuan) – Firefox UX Team from comment #65)
> It's intended to direct the users back to the prompt after in-depth reading
> on the policy content. In comparison, a link to the actual website in a new
> tab, will direct the users away. Given the fact that content is full-screen
> on metro, it's highly possible that the users will not come back to choose
> their preference once they are directed away.

I totally understand that, the lightbox approach is what we desire. However we may need to make a trade off for fx28 if we can't get the online content displaying. I'm not sure what the issues are with getting the lightbox going for fx28, but we should have a fallback in place if for some reason we can't get that going by the time we merge to beta in January. My bet is though we'll be able to get it working.

Tim, what are the issues with this as you see them? Do we have any major road blocks to getting the lightbox working my the beta28 merge?
Maybe I'm jumping the gun here though, if we inline the policy, would legal want it to be displayed in english regardless of the language we run on? If that's the case then there's no harm in inlining the text and bailing on the online content approach until someone demands we update it. We can add a localization note stating the text should not be translated.
(In reply to Jim Mathies [:jimm] from comment #67)
> Maybe I'm jumping the gun here though, if we inline the policy, would legal
> want it to be displayed in english regardless of the language we run on? If
> that's the case then there's no harm in inlining the text and bailing on the
> online content approach until someone demands we update it. We can add a
> localization note stating the text should not be translated.

Or we can avoid to expose these strings at all (or maybe just a note explaining that the policy is in English and the reason for that). 

Web approach has the advantage to let us start localizing that content whenever we want and only for locales who want to do that.
I should have been more clear.

There are string changes that absolutely need to make it in before Aurora. Those amount to basically everything in the mockup except for the lightbox.

For the lightbox:
  The *right* solution (I think) would be to check the value of this pref [1], load that URL in an XHR, get the contents of the <div id="content">, and populate the lightbox with those contents. In that implementation, localizations of the privacy policy will become available in our localized builds as they are created by legal, without requiring code changes or localizer effort. If we go with that implementation, we have unanswered questions that we would need to resolve - "What should we show while we're trying to load the privacy policy page? What should we show if we're not able to load the privacy policy page?" - and we'd have to implement and review the changes. Thus, my plan in the super-short-term was to simply copy+paste the inner HTML of that div into browser.xul and file a follow-up bug to switch us to the dynamic implementation. This super-short-term solution has the benefit that localizers don't ever see the privacy policy text (and so won't start localizing our entire privacy policy), and that our strings that *do* require localization get landed sooner. The downside is that if we want strings for the failure cases mentioned above (the "what if we're unable to show the privacy policy?"), we'll have to implement "the right implementation" in m-c and let it ride the trains, rather than fixing it in Aurora.

[1] https://mxr.mozilla.org/mozilla-central/source/browser/app/profile/firefox.js?rev=dcfc0e0e663d#952
Your short term fix sounds good to me, although can we keep the text out of browser.xul and load it from someplace else when we display the lightbox? That way we don't have to parse it when the browser opens.
(In reply to Jim Mathies [:jimm] from comment #70)
> Your short term fix sounds good to me, although can we keep the text out of
> browser.xul and load it from someplace else when we display the lightbox?
> That way we don't have to parse it when the browser opens.

Yes, sorry, I meant crashprompt.xhtml
Attached patch Part 1 patch v8 (obsolete) — Splinter Review
Rebased, updated strings, added privacy policy to lightbox.
Attachment #828137 - Attachment is obsolete: true
Attachment #8343293 - Flags: review?(jmathies)
Attached patch Part 2 patch v4 (obsolete) — Splinter Review
Rebased
Attachment #8336477 - Attachment is obsolete: true
Attachment #8343294 - Flags: review?(jmathies)
Attached image Screenshots with patches applied (obsolete) —
This is what the crash report page looks like with the patches applied. Yuan, does this look about right?

I'll file follow-ups for making the lightbox text dynamically load from the privacy policy page, and for changing the "Include URLs" toggle to a checkbox.
Attachment #788136 - Attachment is obsolete: true
Attachment #827542 - Attachment is obsolete: true
Attachment #8343296 - Flags: feedback?(ywang)
(In reply to Jim Mathies [:jimm] from comment #67)
> Maybe I'm jumping the gun here though, if we inline the policy, would legal
> want it to be displayed in english regardless of the language we run on? If
> that's the case then there's no harm in inlining the text and bailing on the
> online content approach until someone demands we update it. We can add a
> localization note stating the text should not be translated.

I thin that's a good strategy. From what I've heard, legal prefers the policy to be displayed in English at this moment. Currently they don't have localized versions of public policy. It seemed like a complicate procedure as far as I've learnt. So I don't think in the near term, there will be localized versions of policy available.
Comment on attachment 8343294 [details] [diff] [review]
Part 2 patch v4

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

::: browser/metro/base/content/browser-ui.js
@@ +614,5 @@
>        if (Services.prefs.getBoolPref("app.crashreporter.autosubmit")) {
>          Util.dumpLn("Submitting last crash id:", lastCrashID);
> +        let params = {};
> +        if (!Services.prefs.getBoolPref("app.crashreporter.submitURLs")) {
> +          Util.dumpLn("Stripping URL");

nit - probably don't need this in here.
Attachment #8343294 - Flags: review?(jmathies) → review+
Attachment #8343293 - Flags: review?(jmathies) → review+
(In reply to Tim Abraldes [:TimAbraldes] [:tabraldes] from comment #74)
> Created attachment 8343296 [details]
> Screenshots with patches applied
> 
> This is what the crash report page looks like with the patches applied.
> Yuan, does this look about right?
> 
> I'll file follow-ups for making the lightbox text dynamically load from the
> privacy policy page, and for changing the "Include URLs" toggle to a
> checkbox.

Some visual tweaks:
1. The labels on the actions buttons should be white #FFFFFF. Blue button and triangle color: #649FEF. Grey button over:  #B0B0B0.

2. Make the link "Nightly privacy policy" with no underline until the link is pressed down or hovered over. Change the text color to: #649FEF. 

3. For the header "Would you like to...", use #737980. The grey divider: #C7C7C7. 

Thanks Tim!
Not sure what caused the colors to get all out of whack. With previous patches applied the colors looked like you describe (see obsolete screenshot attachments). Investigating.
Updated patches with the visual tweaks. This screenshot shows what the crash prompt page looks like with the patches applied, and also shows what the buttons look like when hovered (top button is not hovered, bottom button is hovered).

Updated patches incoming.
Attachment #8343296 - Attachment is obsolete: true
Attachment #8343296 - Flags: feedback?(ywang)
Attachment #8343538 - Flags: feedback?(ywang)
Attached patch Part 1 patch v9 (obsolete) — Splinter Review
Fixed colors, carrying forward r+
Attachment #8343293 - Attachment is obsolete: true
Attachment #8343539 - Flags: review+
Attached patch Part 2 patch v5 (obsolete) — Splinter Review
(In reply to Jim Mathies [:jimm] from comment #76)
> ::: browser/metro/base/content/browser-ui.js
> @@ +614,5 @@
> >        if (Services.prefs.getBoolPref("app.crashreporter.autosubmit")) {
> >          Util.dumpLn("Submitting last crash id:", lastCrashID);
> > +        let params = {};
> > +        if (!Services.prefs.getBoolPref("app.crashreporter.submitURLs")) {
> > +          Util.dumpLn("Stripping URL");
> 
> nit - probably don't need this in here.

Removed!

Also:
 - Updated the "include URLs" pref to default to false, even on Aurora and Nightly, per discussion in #windev
 - Removed the links to previous versions of the privacy policy from the lightbox, per discussion in #windev
Attachment #8343294 - Attachment is obsolete: true
Attachment #8343540 - Flags: review+
Blocks: 947077
Blocks: 947078
Blocks: 947081
Filed some follow-up bugs:
 bug 947081 - This is for any follow-up of visual styles that needs to happen in the crash prompt page. Yuan; if there are any remaining changes that need to be made (colors, sizes, locations, etc), please note them in bug 947081
 bug 947077 - This bug is for dynamically loading the content of the privacy policy into the lightbox
 bug 947078 - This bug is for changing the "include URLs" option in the options flyout from a <settings> element to a checkbox


Landed patches on fx-team:
 Part 1 - https://hg.mozilla.org/integration/fx-team/rev/e068a99474a6
 Part 2 - https://hg.mozilla.org/integration/fx-team/rev/a5e4cf95c43e
Backed out due to test failures:
  part 1 - https://hg.mozilla.org/integration/fx-team/rev/80325b9b6854
  part 2 - https://hg.mozilla.org/integration/fx-team/rev/a213a3eef80d

Apparently the browser_crashprompt.js test doesn't run when I do "mach mochitest-metro" so I missed this test failure locally. I'll have to investigate and fix this tomorrow (Friday, December 6th) since it's past my bedtime already.
This patch consolidates the previous two patches and fixes the browser_crashprompt.js test.

Carrying forward r+
Attachment #8343539 - Attachment is obsolete: true
Attachment #8343540 - Attachment is obsolete: true
Attachment #8344242 - Flags: review+
Blocks: 947659
https://hg.mozilla.org/mozilla-central/rev/b44cec692a98
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 28
\o/
For verifying this fix, I think you can use https://code.google.com/p/crashme/ to intentionally crash Metro Firefox.  Install it using desktop Firefox, then switch to Metro and choose the "Crash Now" entry from the "Settings" charm.
Comment on attachment 8343538 [details]
Screenshot with patches applied

Clearing feedback flag
Attachment #8343538 - Flags: feedback?(ywang)
Thanks Matt for the steps from comment 89!

While trying to verify this, on a Win 8 64-bit machine, for iteration #20, with latest Nightly (build ID: 20131212030202) I get some confusing results:

1) if I install using desktop Firefox the add-on from http://people.mozilla.com/~tmielczarek/crashme/ it has 0.4.1 as version, the "Crash Now" entry from the "Settings" charm is visible; then, from Metro mode, I only set 'app.crashreporter.autosubmit' to false (because  'app.crashreporter.prompted'  is already set to false by default); afterwards I click the "Crash Now" entry, and Metro closes; when I restart, I can't see the crash dialog

2) if I install using desktop Firefox the add-on from https://code.google.com/p/crashme/ it has 0.4 as version, the "Crash Now" entry from the "Settings" charm it's not visible 

I know that we disabled desktop addons in metrofx in bug https://bugzilla.mozilla.org/show_bug.cgi?id=946296 so perhaps I should retest this tomorrow or Monday.

My question is: which Crash me version is the one to reliably test with? The one from: https://code.google.com/p/crashme/ ?
Flags: needinfo?(tabraldes)
Hey Manuela, version 0.4.1 is definitely the version you should be using (any version that creates the "Crash Now" entry in the "Settings" charm is good).

> 1) if I install using desktop Firefox the add-on from
> http://people.mozilla.com/~tmielczarek/crashme/ it has 0.4.1 as version, the
> "Crash Now" entry from the "Settings" charm is visible; then, from Metro
> mode, I only set 'app.crashreporter.autosubmit' to false (because 
> 'app.crashreporter.prompted'  is already set to false by default);
> afterwards I click the "Crash Now" entry, and Metro closes; when I restart,
> I can't see the crash dialog

I'll see if I can reproduce this. Do any errors appear in the error console (ctrl+shift+J)?
Flags: needinfo?(tabraldes)
At first I thought I was reproducing the issue, but it turns out I had just not enabled the crash reporter for my build (add "Enabled=1" to the "CrashReporter" section in obj/dist/bin/metro/metroapp.ini). Once I fixed that I saw the crash reporter prompt every time I used the "Crash Now" button. The crash reporter should be enabled by default in Nightly builds I believe, so this probably isn't the same issue you're seeing, Manuela. Just to be sure though, could you check the file at "installdir/metro/metroapp.ini" to see if it indicates that the crash reporter is enabled?
Attached file details.zip
I've retried the below steps, on the same Win 8 64-bit machine, with latest Nightly 29.0a1 (build ID: 20131212030202) and I get the same results

> 1) if I install using desktop Firefox the add-on from
> http://people.mozilla.com/~tmielczarek/crashme/ it has 0.4.1 as version, the
> "Crash Now" entry from the "Settings" charm is visible; then, from Metro
> mode, I only set 'app.crashreporter.autosubmit' to false (because 
> 'app.crashreporter.prompted'  is already set to false by default);
> afterwards I click the "Crash Now" entry, and Metro closes; when I restart,
> I can't see the crash dialog

I've checked the file at "installdir/metro/metroapp.ini" and it seems that it indicates that the crash reporter is enabled. 

Please see the attached .zip, with the next details: the "metroapp.ini" file and 2 screenshots showing the warnings and errors from the Error Console.
Using the latest nightly:
  29.0a1 (2013-12-13)
  Built from http://hg.mozilla.org/mozilla-central/rev/8b5875dc7e31

I followed these steps:
  1. Installed Nightly
  2. Opened desktopFx, set as default
  3. Switched to metroFx
  4. Opened some tabs
  5. Used the "Crash Now" entry in the Settings panel
  6. Set the MetroLastAHE registry entry to 1, to work around bug 950288 (not sure if this step necessary)
  7. Launched metroFx

At this point I saw the session correctly restored and saw the crash prompt appear.

I'm not sure why we're seeing different behavior. Manuela, can you try a couple more things:
  1. Verify in "about:buildconfig" that "--enable-crashreporter" appears on that page
  2. Try with a new build?

Thanks!
I've retested this, and now I can't even see the "Crash Now" entry in the Settings panel. Please see the below steps that I've followed:


on Win 8 64-bit, with latest 32-bit Nightly from ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-central/ (build ID: 20131215030202) I did the following:

1) uninstalled Nightly from my PC

>   1. Installed Nightly (from the above mentioned URL)
>   2. Opened desktopFx, set as default

    2b) Installed using desktop Firefox the add-on from http://people.mozilla.com/~tmielczarek/crashme/ which is the 0.4.1 version

>   3. Switched to metroFx
>   4. Opened some tabs
>   5. Used the "Crash Now" entry in the Settings panel

    I can't see this entry

>   6. Set the MetroLastAHE registry entry to 1, to work around bug 950288
> (not sure if this step necessary)

    How can I do this?

>   7. Launched metroFx


>1. Verify in "about:buildconfig" that "--enable-crashreporter" appears on that page

I've checked this, and it's there.
Depends on: 951802
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: