Convert most Advanced pane dialogs to be in-content

VERIFIED FIXED in Firefox 51

Status

()

defect
P3
normal
VERIFIED FIXED
5 years ago
Last year

People

(Reporter: Paenglab, Assigned: jyeh)

Tracking

(Blocks 1 bug)

unspecified
Firefox 51
Points:
5
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox38 wontfix, firefox39 affected, firefox40 affected, firefox41 affected, firefox42 affected, firefox51 verified)

Details

Attachments

(2 attachments, 12 obsolete attachments)

87.13 KB, image/png
Details
19.56 KB, patch
MattN
: review+
Details | Diff | Splinter Review
Convert advanced.js to use gSubDialog.
This patch needs bug 1035541 applied first.

I had like for the fonts dialog to remove the width style for connection.xul. The width is also without this style correctly calculated on normal and in-content dialogs.

(In reply to Matthew N. [:MattN] (behind on bugmail) from comment #63)
> Comment on attachment 8448050 [details] [diff] [review]
> inContentDialogs.patch
> 
> Review of attachment 8448050 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/themes/shared/incontentprefs/preferences.css
> @@ +897,5 @@
> > +  min-height: 15em;
> > +}
> > +
> > +#certMgrTabbox {
> > +  -moz-margin-start: 0;
> 
> I don't think specific sub-dialog IDs belong in this file. If you want to
> style the tabbox widgets when they're in a sub-dialog, do so in a more
> generic way.

This is only temporary and will be removed by bug 1014208.
Attachment #8454011 - Flags: review?(MattN+bmo)
I've done a try run with only this patch applied to check if the test failures are still present:

https://tbpl.mozilla.org/?tree=Try&rev=90c1536ea0d8

Matthew, please can you look how they can be fixed? I've never done tests.
Does this also cover the update dialog ?
No, I'm on filing a separate bug. This doesn't work with only exchanging the dialog call.
Comment on attachment 8454011 [details] [diff] [review]
advancedPaneDialogsIncontent.patch

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

(In reply to Richard Marti (:Paenglab) from comment #2)
> I've done a try run with only this patch applied to check if the test
> failures are still present:
> 
> https://tbpl.mozilla.org/?tree=Try&rev=90c1536ea0d8
> 
> Matthew, please can you look how they can be fixed? I've never done tests.

I'm working on the tests. They are expecting windows to be opened and closed but this is no longer the case. I'm almost done converting the 2nd test file.

::: browser/components/preferences/connection.xul
@@ -15,5 @@
>              dlgbuttons="accept,cancel,help"
>              onbeforeaccept="return gConnectionsDialog.beforeAccept();"
>              onload="gConnectionsDialog.checkForSystemProxy();"
>              ondialoghelp="openPrefsHelp()"
> -#ifdef XP_MACOSX

Also remove the leading "*" from the jar.mn line for this file since it no longer needs to be pre-processed. Likewise for other dialogs where you remove the last pre-processor directive. (You will see a build warning otherwise).

::: browser/themes/shared/incontentprefs/preferences.css
@@ +898,5 @@
>    min-height: 15em;
>  }
>  
> +#certMgrTabbox {
> +  -moz-margin-start: 0;

It seems like this problem is more general and would apply to other <tabs> elements so this selector should be more general I think.

@@ +902,5 @@
> +  -moz-margin-start: 0;
> +}
> +
> +#ConnectionsDialog > .prefWindow-dlgbuttons {
> +  padding-bottom: 8px;

I wonder if this style could be more generic as the problem might occur with all <prefwindow> maybe?

I'm trying to avoid putting much single-dialog-specific stuff in this file as those styles are better in the CSS for that dialog itself except when they affect multiple sub-dialogs and the same changes would need to be made for more than one.
Attachment #8454011 - Flags: review?(MattN+bmo) → review-
(In reply to Matthew N. [:MattN] (behind on bugmail) from comment #5)
> Comment on attachment 8454011 [details] [diff] [review]
> advancedPaneDialogsIncontent.patch
> 
> Review of attachment 8454011 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'm working on the tests. They are expecting windows to be opened and closed
> but this is no longer the case. I'm almost done converting the 2nd test file.

Thanks

> ::: browser/components/preferences/connection.xul
> @@ -15,5 @@
> >              dlgbuttons="accept,cancel,help"
> >              onbeforeaccept="return gConnectionsDialog.beforeAccept();"
> >              onload="gConnectionsDialog.checkForSystemProxy();"
> >              ondialoghelp="openPrefsHelp()"
> > -#ifdef XP_MACOSX
> 
> Also remove the leading "*" from the jar.mn line for this file since it no
> longer needs to be pre-processed. Likewise for other dialogs where you
> remove the last pre-processor directive. (You will see a build warning
> otherwise).

I leaved the "*" because the MPL header is still processed. Should I change the header to remove the preprocessor switch or leave it?

> ::: browser/themes/shared/incontentprefs/preferences.css
> @@ +898,5 @@
> >    min-height: 15em;
> >  }
> >  
> > +#certMgrTabbox {
> > +  -moz-margin-start: 0;
> 
> It seems like this problem is more general and would apply to other <tabs>
> elements so this selector should be more general I think.

I've made it more general.

> @@ +902,5 @@
> > +  -moz-margin-start: 0;
> > +}
> > +
> > +#ConnectionsDialog > .prefWindow-dlgbuttons {
> > +  padding-bottom: 8px;
> 
> I wonder if this style could be more generic as the problem might occur with
> all <prefwindow> maybe?

It now applies to all prefwindow. I had to remove the prefwindow's padding-bottom to not have double padding.

I moved the "End sub-dialog" comment below the ":-moz-any(dialog, window, prefwindow) groupbox" rule because this rule also applies to the dialogs. I may remove this depending in which order the patches land.
Attachment #8454011 - Attachment is obsolete: true
Attachment #8454637 - Flags: review?(MattN+bmo)
Updated to tip.
Attachment #8454637 - Attachment is obsolete: true
Attachment #8454637 - Flags: review?(MattN+bmo)
Attachment #8460306 - Flags: review?(MattN+bmo)
Removed the now no more needed "dialog tabs" rule after landing of bug 1038288.
Attachment #8460306 - Attachment is obsolete: true
Attachment #8460306 - Flags: review?(MattN+bmo)
Attachment #8461702 - Flags: review?(MattN+bmo)
Comment on attachment 8461702 [details] [diff] [review]
advancedPaneDialogsIncontent.patch

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

r=me with comments

::: browser/components/preferences/connection.xul
@@ +15,5 @@
>              dlgbuttons="accept,cancel,help"
>              onbeforeaccept="return gConnectionsDialog.beforeAccept();"
>              onload="gConnectionsDialog.checkForSystemProxy();"
>              ondialoghelp="openPrefsHelp()"
> +            style="">

See bug 1036018 comment 2. I guess this isn't a problem anymore.

@@ -15,5 @@
>              dlgbuttons="accept,cancel,help"
>              onbeforeaccept="return gConnectionsDialog.beforeAccept();"
>              onload="gConnectionsDialog.checkForSystemProxy();"
>              ondialoghelp="openPrefsHelp()"
> -#ifdef XP_MACOSX

Since this was the last preprocessor directive in the file, remove the "*" from this line in jar.mn to avoid a build warning.

::: browser/components/preferences/in-content/advanced.js
@@ +805,5 @@
>     * Displays the user's certificates and associated options.
>     */
>    showCertificates: function ()
>    {
> +    gSubDialog.open("chrome://pippki/content/certManager.xul");

This dialog gets clipped on the right for OS X so perhaps move this to a new bug and have the commit message/summary say "some advanced pane…"?

::: browser/themes/shared/incontentprefs/preferences.css
@@ +865,5 @@
> +  padding-bottom: 0;
> +}
> +
> +prefwindow > .prefWindow-dlgbuttons {
> +  padding-bottom: 10px;

Are these to fix clipping or do align to project chameleon? If the dialog isn't really ugly and is still usable without this, I'd rather we wait for bug 1036090 to make the styling changes.
Attachment #8461702 - Flags: review?(MattN+bmo) → review+
(In reply to Matthew N. [:MattN] from comment #9)
> Since this was the last preprocessor directive in the file, remove the "*"
> from this line in jar.mn to avoid a build warning.

Note that the warning is about there being no useful preprocessor directives so you need to remove the mode-line and convert the license to <!-- style from https://www.mozilla.org/MPL/headers/
Reminder for myself to finish fixing the tests.
Status: NEW → ASSIGNED
Flags: needinfo?(MattN+bmo)
(In reply to Matthew N. [:MattN] from comment #9)
> Comment on attachment 8461702 [details] [diff] [review]
> advancedPaneDialogsIncontent.patch
> 
> Review of attachment 8461702 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me with comments
> 
> ::: browser/components/preferences/connection.xul
> @@ +15,5 @@
> >              dlgbuttons="accept,cancel,help"
> >              onbeforeaccept="return gConnectionsDialog.beforeAccept();"
> >              onload="gConnectionsDialog.checkForSystemProxy();"
> >              ondialoghelp="openPrefsHelp()"
> > +            style="">
> 
> See bug 1036018 comment 2. I guess this isn't a problem anymore.

Removed the style=""

> ::: browser/components/preferences/in-content/advanced.js
> @@ +805,5 @@
> >     * Displays the user's certificates and associated options.
> >     */
> >    showCertificates: function ()
> >    {
> > +    gSubDialog.open("chrome://pippki/content/certManager.xul");
> 
> This dialog gets clipped on the right for OS X so perhaps move this to a new
> bug and have the commit message/summary say "some advanced pane…"?

This is again a OS X only issue. I'll file a bug for making this dialog in-content.

> ::: browser/themes/shared/incontentprefs/preferences.css
> @@ +865,5 @@
> > +  padding-bottom: 0;
> > +}
> > +
> > +prefwindow > .prefWindow-dlgbuttons {
> > +  padding-bottom: 10px;
> 
> Are these to fix clipping or do align to project chameleon? If the dialog
> isn't really ugly and is still usable without this, I'd rather we wait for
> bug 1036090 to make the styling changes.

It was a clipping fix. I removed it now.
Attachment #8461702 - Attachment is obsolete: true
Attachment #8467882 - Flags: review+
Updated to tip.
Attachment #8467882 - Attachment is obsolete: true
Attachment #8475435 - Flags: review+
Is this patch waiting on something else to land ?
See comment 12 for fixing tests.
Blocks: 993383
Duplicate of this bug: 993383
This should block shipping the in-content prefs because it fixes bug 993383 as well.
Blocks: ship-incontent-prefs
No longer blocks: 752197, 993383
Points: --- → 3
Priority: -- → P3
Comment on attachment 8475435 [details] [diff] [review]
advancedPaneDialogsIncontent.patch

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

::: browser/locales/en-US/chrome/browser/preferences/connection.dtd
@@ -4,5 @@
>  
>  
>  <!ENTITY  connectionsDialog.title       "Connection Settings">
> -<!ENTITY  window.width                  "37em">
> -<!ENTITY  window.macWidth               "39em">

the patch that gets uplifted should leave these strings here so as not to modify a strings file on an string-frozen branch.
In our meeting today Gijs said that if he can get to it before Thursday that he will try to help here.
Flags: needinfo?(gijskruitbosch+bugs)
Richard, maybe this is enough for you to run with?

I don't remember what else there was to do. I think there may still be more tests in the directory to fix.
Flags: needinfo?(MattN+bmo) → needinfo?(richard.marti)
Flags: needinfo?(gijskruitbosch+bugs)
This is what I get with your patch applied: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d2613bdedd43
Flags: needinfo?(richard.marti) → needinfo?(MattN+bmo)
OK, there are still some failures which is kinda what I expected since it's a work-in-progress (WIP) patch. I was wondering if you would be able to finish it by looking at what it's changing.
Flags: needinfo?(MattN+bmo) → needinfo?(richard.marti)
(In reply to Matthew N. [:MattN] from comment #24)
> OK, there are still some failures which is kinda what I expected since it's
> a work-in-progress (WIP) patch. I was wondering if you would be able to
> finish it by looking at what it's changing.

I'm sorry, I checked the failing files and my JS knowledge so limited that don't see at all why it's failing. And I also don't really see what it is doing when it is working.
Flags: needinfo?(richard.marti)
Posted patch Fix connection tests (obsolete) — Splinter Review
This fixes the first test. I couldn't fix the second one though.
Attachment #8592300 - Attachment is obsolete: true
Attachment #8595906 - Attachment is patch: true
(In reply to Tim Nguyen [:ntim] from comment #26)
> Created attachment 8595906 [details] [diff] [review]
> Fix connection tests
> 
> This fixes the first test. I couldn't fix the second one though.

It looks like the window corresponds to about:blank, which means the test runs before the dialog loads. This should be easy to fix, though I don't have much knowledge about JS generators.
At this point we're gonna hold on fixing this bug for 38 as the risk is pretty high and we won't have much time to get fixes in if some of the dialogs don't size correctly or have issues with saving their values.

If this lands soon we could still try to get it uplifted to 39.
Whiteboard: [sf-hackweek]
Points: 3 → 5
Posted patch Patch with tests fixed (obsolete) — Splinter Review
I guess I shouldn't have folded the patches together, since it may now be harder to review. I fixed up browser_connection_bug388287.js and browser_proxy_backup.js so that they now pass.

Browser Chrome Test Summary
        Passed: 1135
        Failed: 0
        Todo: 0
Attachment #8475435 - Attachment is obsolete: true
Attachment #8595906 - Attachment is obsolete: true
Attachment #8596081 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8596081 [details] [diff] [review]
Patch with tests fixed

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

::: browser/components/preferences/connection.xul
@@ -18,5 @@
> -            ondialoghelp="openPrefsHelp()"
> -#ifdef XP_MACOSX
> -            style="width: &window.macWidth; !important;">
> -#else
> -            style="width: &window.width; !important;">

This affects l10n. Why was it removed?

::: browser/components/preferences/in-content/tests/browser_connection.js
@@ +35,5 @@
> +    registerCleanupFunction(() => {
> +      gBrowser.removeCurrentTab();
> +    });
> +    let dialog = yield gBrowser.contentWindow.gAdvancedPane.showConnections();
> +    ok(true, "connection window opened");

Maybe ok(dialog) instead?

::: browser/components/preferences/in-content/tests/browser_connection_bug388287.js
@@ +7,2 @@
>  Components.utils.import("resource://gre/modules/Services.jsm");
> +Components.utils.import("resource://gre/modules/Task.jsm");

So err, this test... what does it do? It doesn't seem to actually test anything. Reading the original bug here, it seems like the connections dialog shouldn't close for the invalid values, but I fail to see anything in the test that actually verifies that - bug 388287 comment 21 just says "the test will fail if the dialog closes when it's not supposed to", but I don't see where that would happen...

The old test checked for the dialog closing (using ok(closeable)), but it seems the new incarnation doesn't anymore.

@@ +153,3 @@
>  
> +function promiseDialogClosing(dialog) {
> +  return waitForEvent(dialog, "dialogclosing");

This doesn't get used in this test.

::: browser/components/preferences/in-content/tests/browser_proxy_backup.js
@@ +35,3 @@
>    // instantApply must be true, otherwise connection dialog won't save
>    // when opened from in-content prefs
>    Services.prefs.setBoolPref("browser.preferences.instantApply", true);

This should be obsolete now. If it isn't then we have a problem, and we should fix it. So please remove it.

@@ +53,5 @@
> +        try {
> +          yield test.run();
> +        } finally {
> +          if (test.teardown) {
> +            yield test.teardown();

If we want to do this, can we pull it out into head.js ? It doesn't make much sense in this particular test on its own, and it'll reduce duplication.

@@ +89,5 @@
> +}
> +
> +function dialogClosingCallback(aPromise, aEvent) {
> +  // Wait for the close handler to unload the page
> +  waitForEvent(content.gSubDialog._frame, "load", 4000).then((aEvt) => {

4000 is a beautifully smelly magical number. Why do we need it?

@@ +96,5 @@
> +    ise(content.gSubDialog._frame.getAttribute("style"), "",
> +        "Check that inline styles were cleared");
> +    ise(content.gSubDialog._frame.contentWindow.location.toString(), "about:blank",
> +       "Check the sub-dialog was unloaded");
> +    if (gTeardownAfterClose) {

Nothing sets this to true in this test. If we're copy-pasting from the other one, can we move this to head.js as well? Probably need to define gTeardownAfterClose in there and set it to false in there, too.
Attachment #8596081 - Flags: review?(gijskruitbosch+bugs) → feedback+
Blocks: 1172207
Richard, are you still working on this bug?
Jaws and MattN worked on the tests as I'm a noob in tests writing/fixing. I'm sure they have nothing against if you try to finish this work.
Thanks. I guess Jaws then needs to answer the questions from comment 30 first...
Assignee: richard.marti → jaws
Can we expect to have this landed at some point ?
Flags: needinfo?(jaws)
This will probably be done in the next revision to the preferences, but I haven't seen that mentioned in any near-term planning. It will probably remain as-is for the time being unless someone volunteers to undertake the reorganization of the preferences (a larger task and one that will likely involve multiple stakeholders).
Flags: needinfo?(jaws)
Assignee: jaws → nobody
Status: ASSIGNED → NEW
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #35)
> This will probably be done in the next revision to the preferences, but I
> haven't seen that mentioned in any near-term planning. It will probably
> remain as-is for the time being unless someone volunteers to undertake the
> reorganization of the preferences (a larger task and one that will likely
> involve multiple stakeholders).

Sorry, I mistook this bug for one that is requesting that the subpanes within the Advanced pane be separated out. This bug doesn't need to be part of a larger task, but I don't have time right now to finish this patch.
Blocks: 752197
Trying to figure out how many works need to be done to finish the patch.
Assignee: nobody → jyeh
Posted patch wip-bug-1036815.patch (obsolete) — Splinter Review
Hi Matthew, I made this patch with test fixed, but I am not sure if I fix them correctly. And feel free to correct me if I missed anything in this patch.

Thanks!
Attachment #8596081 - Attachment is obsolete: true
Attachment #8776525 - Flags: review?(MattN+bmo)
Comment on attachment 8776525 [details] [diff] [review]
wip-bug-1036815.patch

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

::: browser/components/preferences/in-content/tests/browser_connection_bug388287.js
@@ +69,2 @@
>      ftpPortPref.value = 0;
>      doc.documentElement.acceptDialog();

Can we test that the dialog doesn't close like it did before? This is what Gijs was talking about in a previous review I think.

@@ -116,5 @@
>      ftpPortPref.value = 0;
>      doc.documentElement.acceptDialog();
>  
> -    // From now on, the dialog should close since we are giving it legitimate inputs.
> -    // The test will timeout if the onbeforeaccept kicks in erroneously.

Why did you remove this comment?

@@ +72,4 @@
>      // Both ports 80, share on
>      httpPortPref.value = 80;
>      ftpPortPref.value = 80;
>      doc.documentElement.acceptDialog();

For the tests that are supposed to close the dialog, I think we should yield on a promise that the domwindowclosed occurs before continuing.
Attachment #8776525 - Flags: review?(MattN+bmo)
Posted patch wip-bug-1036815.patch (obsolete) — Splinter Review
Thanks for the feedback. I finally realize some intentions in the test.

I update the test with waiting for the `dialogclosing` event when the dialog is closing.

However, for the test that we should fail if the dialog closes when it's not supposed to, I couldn't find a way to test it with in-content dialog. If we yield on a promise for the `dialogclosing` event in this case, the promise will fail to resolve.

Do you have any idea for this kind of test?
Attachment #8776525 - Attachment is obsolete: true
Attachment #8779287 - Flags: feedback?(MattN+bmo)
Posted patch wip-bug-1036815.patch (obsolete) — Splinter Review
Updated to tip.
Attachment #8779287 - Attachment is obsolete: true
Attachment #8779287 - Flags: feedback?(MattN+bmo)
Attachment #8781509 - Flags: feedback?(MattN+bmo)
Comment on attachment 8781509 [details] [diff] [review]
wip-bug-1036815.patch

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

I think the only remaining blocking problem is browser_connection_bug388287.js

::: browser/components/preferences/in-content/tests/browser_connection.js
@@ +22,4 @@
>  
>    // instantApply must be true, otherwise connection dialog won't save
>    // when opened from in-content prefs
>    Services.prefs.setBoolPref("browser.preferences.instantApply", true);

Is this still necessary?

::: browser/components/preferences/in-content/tests/browser_connection_bug388287.js
@@ +39,5 @@
>  
>      // Convenient function to reset the variables for the new window
> +    function* setDoc() {
> +      if (closeable) {
> +        let dialogClosingEvent = yield dialogClosingPromise;

This is going to be resolved to the first resolution and it doesn't seem like it will reflect reality for the tests afterwards.
Attachment #8781509 - Flags: feedback?(MattN+bmo) → feedback+
Patch updated as I forgot to reset the dialogClosingPromise in the previous patch. The unused instantApply also removed.
Attachment #8781509 - Attachment is obsolete: true
Attachment #8781921 - Flags: review?(MattN+bmo)
Comment on attachment 8781921 [details] [diff] [review]
Bug-1036815-Convert-Advanced-pane-dialogs-to-be-in-content.patch

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

Thanks. It might be worth getting QE verification in locales with longer strings to verify nothing is cropped in the 3 dialogs.
Attachment #8781921 - Flags: review?(MattN+bmo) → review+
Verify in a locale with long strings (e.g. German?) that the Connections, Offline Storage Exceptions, and Device Manager dialogs (all found in the Advanced section of preferences) look normal with no cropped text in a new profile with the default subdialog sizes.
Flags: qe-verify+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/fx-team/rev/20e45f63c2e6
Convert Advanced pane dialogs to be in-content. r=MattN
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/20e45f63c2e6
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
Duplicate of this bug: 1190163
Hi. The certificate manager in advanced pane is not in-content (even though attached screenshot indiates so).

Looking at the code, it is still openDialog("chrome://pippki/content/certManager.xul",
(In reply to Hussam Al-Tayeb from comment #50)
> Hi. The certificate manager in advanced pane is not in-content (even though
> attached screenshot indiates so).
> 
> Looking at the code, it is still
> openDialog("chrome://pippki/content/certManager.xul",

This is bug 1049001
(In reply to Matthew N. [:MattN] from comment #45)
> Verify in a locale with long strings (e.g. German?) that the Connections,
> Offline Storage Exceptions, and Device Manager dialogs (all found in the
> Advanced section of preferences) look normal with no cropped text in a new
> profile with the default subdialog sizes.

It seems that after this change text gets cropped and overflowed in Polish (pl) nightly builds.
Flags: needinfo?(francesco.lodolo)
(In reply to Stefan Plewako [:stef] from comment #52)
> It seems that after this change text gets cropped and overflowed in Polish
> (pl) nightly builds.

Please file a bug blocking this one.
Flags: needinfo?(francesco.lodolo)
(In reply to Francesco Lodolo [:flod] from comment #53)
> Please file a bug blocking this one.

(sorry for the double comment) Adding a screenshot would help. I don't see scrolling in Italian, changing some of the radio buttons doesn't cause horizontal scrolling (they wrap), changing the intro text at the top does it.
Depends on: 1298872
(Updating summary to reflect that only most of the dialogs were converted per comment 50 and comment 51.)
Summary: Convert Advanced pane dialogs to be in-content → Convert most Advanced pane dialogs to be in-content
Blocks: 1239523
I have reproduced this on Firefox nightly according to(2014-07-10)

Fixing bug is verified on Latest Nightly--Build ID: (20160908030434), User Agent: Mozilla/5.0 (Windows NT 10.0; rv:51.0) Gecko/20100101 Firefox/51.0


Tested OS-- Windows10 32bit
QA Whiteboard: [testday-20160909]
Blocks: 1204320
Depends on: 1336141
No longer depends on: 1429909
You need to log in before you can comment on or make changes to this bug.