Open
Bug 631304
Opened 14 years ago
Updated 2 years ago
Unknown content type dialog sometimes show simple ui even it states to show the normal ui
Categories
(Toolkit :: Downloads API, defect)
Toolkit
Downloads API
Tracking
()
NEW
People
(Reporter: u279076, Unassigned)
References
Details
(Whiteboard: [mozmill-test-blocked])
Attachments
(2 files, 1 obsolete file)
Currently, in downloadManager.downloadFileOfUnkownType() we only wait for the Download Manager dialog to open. We don't verify the loading of its content. As a result, we fail when an empty dialog appears. While this happens rarely, it does happen (see screenshot).
NOTE: This came out of bug 631246.
Comment 1•14 years ago
|
||
With my fix for the mozmill internal window.loaded checks it should normally not happen. When we are searching for the window do we use handleWindow or simply a getMostRecentWindow? We have to wait for window.documentLoaded here.
We use getMostRecentWindow before calling handleWindow:
controller.waitForEval("subject.getMostRecentWindow('').document.documentElement.id == 'unknownContentType'", gTimeout, 100, mozmill.wm);
I did a test by dumping documentLoaded between the waitForEval and handleWindow. In the case of the empty dialog, documentLoaded is still TRUE.
Also, adding a sleep(100) here does not prevent the empty dialog from occurring.
(In reply to comment #4)
> Also, adding a sleep(100) here does not prevent the empty dialog from
> occurring.
Bumping up the number to 1000 (a full second) seems to have prevented this dialog from happening. It's far from ideal, but I'm fine with checking this in as a temporary workaround.
Henrik, what do you think?
(In reply to comment #5)
> (In reply to comment #4)
> > Also, adding a sleep(100) here does not prevent the empty dialog from
> > occurring.
>
> Bumping up the number to 1000 (a full second) seems to have prevented this
> dialog from happening. It's far from ideal, but I'm fine with checking this in
> as a temporary workaround.
>
> Henrik, what do you think?
Ugh, scratch that...I did a second testrun of 100 and it happened this time...so after 147 testruns it happens once. A sleep(1000) simply makes it happen far less often.
I'm wondering if we shouldn't bring in a developer who works on the code in this area. Is it possibly a Firefox bug?
Comment 8•14 years ago
|
||
(In reply to comment #7)
> I'm wondering if we shouldn't bring in a developer who works on the code in
> this area. Is it possibly a Firefox bug?
Sounds like. I think you can ask Shawn Wilsher, or check mxr for the xul/js file. There should be at least some names.
As per email, Shawn, if you have any information we can use to debug what might be happening here, please comment on this bug.
Thanks
Comment 10•14 years ago
|
||
If this is a core issue we should move it accordingly.
Shawn, does this dialog use any timer to lazy display the window content dependent on the MIME type?
Comment 11•14 years ago
|
||
(In reply to comment #10)
> Shawn, does this dialog use any timer to lazy display the window content
> dependent on the MIME type?
It doesn't use a timer, no.
I'm curious as to what you are testing with this though; it should already have coverage through automated tests.
Comment 12•14 years ago
|
||
(In reply to comment #11)
> I'm curious as to what you are testing with this though; it should already have
> coverage through automated tests.
At the moment our Mozmill tests do not support the save file dialog, because those are native. So using the unknown type dialog allows us to run tests for the download manager.
Comment 13•14 years ago
|
||
In mozilla-central, we have this chrome test that opens the dialog:
https://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/downloads/tests/chrome/test_unknownContentType_dialog_layout.xul
Check to make sure you aren't doing something terribly different?
Reporter | ||
Comment 14•14 years ago
|
||
Looking at that test file, I'm wondering if we are triggering the simple UI in some cases:
63 let tests = [
64 { // This URL will trigger the simple UI, where only the Save an Cancel buttons are available
65 url: "http://mochi.test:8888/chrome/toolkit/mozapps/downloads/tests/chrome/unknownContentType_dialog_layout_data.pif",
66 elements: {
67 basicBox: { collapsed: false },
68 normalBox: { collapsed: true }
69 }
70 }
Comment 15•14 years ago
|
||
(In reply to comment #14)
> Looking at that test file, I'm wondering if we are triggering the simple UI in
> some cases:
That should give you some buttons still though.
Reporter | ||
Comment 16•14 years ago
|
||
(In reply to comment #15)
> (In reply to comment #14)
> > Looking at that test file, I'm wondering if we are triggering the simple UI in
> > some cases:
> That should give you some buttons still though.
As per the attached screenshot, we only get the SUBMIT and CANCEL buttons.
Comment 17•14 years ago
|
||
(In reply to comment #13)
> In mozilla-central, we have this chrome test that opens the dialog:
> https://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/downloads/tests/chrome/test_unknownContentType_dialog_layout.xul
>
> Check to make sure you aren't doing something terribly different?
This test doesn't check the radio buttons. Where the code which decides if the simple UI has to be displayed? If we are triggering this behavior some times, it looks like a bug in that code.
Reporter | ||
Comment 18•14 years ago
|
||
From the XUL file it looks like the dialog has two properties, basicBox and normalBox which are true,false respectively for the full UI. I'm wonder if we should/could check for this and enforce it in our tests?
Reporter | ||
Comment 19•14 years ago
|
||
(In reply to comment #18)
> From the XUL file it looks like the dialog has two properties, basicBox and
> normalBox which are true,false respectively for the full UI. I'm wonder if we
> should/could check for this and enforce it in our tests?
Sorry, that should be:
basicBox.collapsed = true and
normalBox.collapsed = false
Comment 20•14 years ago
|
||
We always should get the same type of dialog. There shouldn't be a flip to the basic box under unknown situations.
Reporter | ||
Comment 21•14 years ago
|
||
(In reply to comment #20)
> We always should get the same type of dialog. There shouldn't be a flip to the
> basic box under unknown situations.
Without discounting that, we should at least extend DownloadsAPI to check that state so we can report "full UI" vs "simple UI" to brasstacks.
I agree that we should always be getting the same dialog which should also be taken care of on this bug.
Comment 22•14 years ago
|
||
That should be a fairly easy patch. Do you mind working on it? Beside that we should file a separate bug for the dialog itself.
Reporter | ||
Comment 23•14 years ago
|
||
(In reply to comment #22)
> That should be a fairly easy patch. Do you mind working on it? Beside that we
> should file a separate bug for the dialog itself.
Yup, I can work on that patch. I'll file a new bug for the simple-UI issue.
Comment 24•14 years ago
|
||
(In reply to comment #10)
> Shawn, does this dialog use any timer to lazy display the window content
> dependent on the MIME type?
I forgot that the window is actually opened by a timer:
https://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/downloads/nsHelperAppDlg.js#175
The code to initialize the dialog is here:
https://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/downloads/nsHelperAppDlg.js#404
Comment 25•14 years ago
|
||
That would mean that shouldntRememberChoice and openWithDefaultOK() are the only conditions which let us select the type of box here. We could dump both values of the above items to check which one is causing this problem.
I think we should keep this bug for the helper dialog and file a new one for updating our shared module.
Reporter | ||
Comment 26•14 years ago
|
||
(In reply to comment #25)
> I think we should keep this bug for the helper dialog and file a new one for
> updating our shared module.
I'm confused about what you want here now...
Comment 27•14 years ago
|
||
Lets work out the simple update for the download.js module on bug 631246. This bug already has a lot of valuable information which will help to find the reason for that strange behavior.
Assignee: anthony.s.hughes → nobody
Component: Mozmill Tests → Download Manager
Product: Mozilla QA → Toolkit
QA Contact: mozmill-tests → download.manager
Reporter | ||
Comment 28•14 years ago
|
||
Here is a simplified version of testDownloadStates. Henrik, please test
locally and you will see what I mean.
You should notice that under all conditions (whether we see the Normal UI or
the Simple UI), normalBox.collapsed is always FALSE and basicBox.collapsed is
always TRUE.
Reporter | ||
Comment 29•14 years ago
|
||
Here is a new version which dumps shouldnRememberChoice and openWithDefaultOK.
Here is a sample output:
*** TESTRUN 5 ***
*** MIME-Type: application/x-bzip2
*** isExecutable: false
*** shouldntRememberChoice: false
*** openWithDefaultOK: true
*** EXPECT normalBox.collapsed=FALSE, basicBox.collapsed=TRUE
*** normalBox.collapsed: false
*** basicBox.collapsed: true
*** TESTRUN 6 ***
*** MIME-Type: application/x-bzip2
*** isExecutable: false
*** shouldntRememberChoice: false
*** openWithDefaultOK: true
*** EXPECT normalBox.collapsed=FALSE, basicBox.collapsed=TRUE
*** normalBox.collapsed: false
*** basicBox.collapsed: true
Testrun 5 we get the full UI.
Testrun 6 we get the simple UI.
Both datasets are completely identical.
Attachment #510736 -
Attachment is obsolete: true
Attachment #510788 -
Flags: feedback?(hskupin)
Comment 30•14 years ago
|
||
Comment on attachment 510788 [details]
testDownloadStates-simplified.js v2
Looks fine. Now we only need one instance of a test failure. It would probably worth to run it in a vm where it always happened.
Attachment #510788 -
Flags: feedback?(hskupin) → feedback+
Comment 31•14 years ago
|
||
(In reply to comment #30)
> Looks fine. Now we only need one instance of a test failure. It would probably
> worth to run it in a vm where it always happened.
Scratch that. Missed to look at the former comments.
Updated•14 years ago
|
Summary: Improve DownloadAPI to check for content of Save File dialog → Unknown content type dialog sometimes show simple ui even it states to show the normal ui
Whiteboard: [mozmill-test-blocked]
Reporter | ||
Comment 32•14 years ago
|
||
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/downloads/nsHelperAppDlg.js#500
I'm wondering if this "hack" is failing to work in some cases. Shawn, do you have any advice on how I can debug this?
Comment 33•14 years ago
|
||
Bug 614557 fixed Venkman today. So if you build the extension on your own, you could use it with mozmill, in case we can reproduce it manually. Or another idea, we could create a mozmill test like the testcase on this bug, and check for the simple/normal ui and the radio buttons. If the radio button throw an error lets catch it and call a long sleep so we would have enough time to manually investigate this problem and set breakpoints.
Comment 34•14 years ago
|
||
(In reply to comment #32)
> I'm wondering if this "hack" is failing to work in some cases. Shawn, do you
> have any advice on how I can debug this?
I don't even understand why we'd need that. That code has been there since day one, so there's no bug to look at as to why it was added either :/
Comment 35•14 years ago
|
||
I have uploaded a temporary version of Venkman with the fix on bug 614557 here:
http://people.mozilla.com/~hskupin/downloads/
Lets see if I can come closer to the problem.
Reporter | ||
Comment 36•10 years ago
|
||
Is this bug still worth working on or should we just close it?
Comment 38•6 years ago
|
||
No assignee, updating the status.
Comment 39•6 years ago
|
||
No assignee, updating the status.
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•