Closed Bug 451779 Opened 16 years ago Closed 14 years ago

Sometimes the EULA is not shown when running a major update 2.0.0.16=>3.0.1

Categories

(Toolkit :: Application Update, defect)

1.8 Branch
x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: whimboo, Unassigned)

References

Details

Attachments

(3 files)

Attached image Empty EULA box
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.16) Gecko/20080702 Firefox/2.0.0.16

Testing the major update on the beta channel gives me sometimes a blank page instead of the EULA. With an existing profile it happened on the first try. Further tries showed the EULA again. Further tests with a fresh profiles show the EULA the first time but not for additional tries. The box stays empty even after a restart of Firefox. There is also no error shown within the Error Console. See the attachment which shows the empty box.

We shouldn't deliver a major update where users have to agree to something they cannot read. And I believe this is major in that case.
Try setting app.update.log.all to true in about:config, retest, and report any errors or log messages that are interesting.

Could you also provide exact steps to reproduce just in case?
I don't have real step for the moment. I only tried to run the update several times by opening "Help | Check for Updates...". I'll try to get the full log.
Blocks: 442105
Whiteboard: [MU?]
Version: Trunk → 1.8 Branch
Attached file last-update.log
Doesn't seem to be really helpful information. Robert, anything I can do to give you more information?
The pref I gave you enables reporting to the error console... sorry I wasn't more specific
Doesn't change anything. There is nothing displayed inside the Error Console.

Meanwhile I'm able to reproduce this on another VmWare machine with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.1.16) Gecko/20080702 Firefox/2.0.0.16

Steps to (probably) reproduce:
0. Set the channel to Beta
1. Create a fresh profile
2. Start Firefox
3. Select "Help | Check for Updates..."
4. Click on "Get new version..." if the update was found
=> EULA is shown
5. Cancel the update
6. Select "Help | Check for Updates..."
7. Click again on "Get new version..." if the update was found
=> EULA isn't shown
8. Repeat steps 5-7
=> EULA is never shown again
I should note that I see the spinning ball at the top and some text inside the box for a split second.
When the EULA isn't shown and I use the DOMi to check the dialog, the content appears when I click onto the document node under xul:browser (with class=licenceContent). So we have a refreshing problem here?
(In reply to comment #5)
> Doesn't change anything. There is nothing displayed inside the Error Console.
Probably neither here nor there but you definitely should see messages in the error console from the update service with that pref set to true.

I'm not all that familiar with that code but it could very well be that the content isn't invalidated at the right time consistently.
Does the license show if you bring another window to the front and then bring the wizard to the front?
(In reply to comment #8)
> Probably neither here nor there but you definitely should see messages in the
> error console from the update service with that pref set to true.

Doesn't work. No messages at all. But after activating strict js warnings I get following warnings and one exception:

Warning: trailing comma is not legal in ECMA-262 object initializers
Source File: chrome://mozapps/content/update/updates.js
Line: 520
Source Code:
}

Warning: variable progress hides argument
Source File: chrome://mozapps/content/update/updates.js
Line: 1366, Column: 8
Source Code:
    var progress = Math.round(100 * (progress/maxProgress));

Warning: trailing comma is not legal in ECMA-262 object initializers
Source File: chrome://mozapps/content/update/updates.js
Line: 1688
Source Code:
};

Warning: trailing comma is not legal in ECMA-262 object initializers
Source File: chrome://mozapps/content/update/updates.js
Line: 1716
Source Code:
};


Warning: function set_pageIndex does not always return a value
Source File: chrome://global/content/bindings/wizard.xml
Line: 109, Column: 20
Source Code:
          return val;

Error: [Exception... "'Failure' when calling method: [nsIWritablePropertyBag::getProperty]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://mozapps/content/update/updates.js :: anonymous :: line 725"  data: no]
Source File: chrome://mozapps/content/update/updates.js
Line: 725


Any further try to open the update dialog only shows following:

Warning: function set_pageIndex does not always return a value
Source File: chrome://global/content/bindings/wizard.xml
Line: 109, Column: 20
Source Code:
          return val;

Error: [Exception... "'Failure' when calling method: [nsIWritablePropertyBag::getProperty]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://mozapps/content/update/updates.js :: anonymous :: line 725"  data: no]
Source File: chrome://mozapps/content/update/updates.js
Line: 725


    var licenseAccepted;
    try {
=>    licenseAccepted = update.getProperty("licenseAccepted");
    }
    catch (e) {
      gUpdates.update.setProperty("licenseAccepted", "false");
      licenseAccepted = false;
    }

Looks like something which is probably related to my issue?
Hmmm... that may be a typo in the 1.8 landing from bug 348389... on 1.9 and trunk that line is licenseAccepted = gUpdates.update.getProperty("licenseAccepted");

Not positive that is the cause but it may be
Could you verify the spelling of the pref, it is a boolean pref, and that there isn't a space at the end which sometimes happens when copying / pasting.

app.update.log.all boolean value of true
btw: my main doubt about that exception is that it is surrounded by a try catch which should catch the error so it falls through to the next section... I wouldn't be surprised if that error occurs when the page is displayed as well.
(In reply to comment #12)
> app.update.log.all boolean value of true

That's exactly what I've typed in manually after doing a right click and selecting "New | Boolean". This happens for a fresh profile. So is another pref necessary? Sorry, but I can't get it to work that way. 

(In reply to comment #11)
> Hmmm... that may be a typo in the 1.8 landing from bug 348389... on 1.9 and
> trunk that line is licenseAccepted =
> gUpdates.update.getProperty("licenseAccepted");
> 
> Not positive that is the cause but it may be

I've already changed that, but it still doesn't work. So it's probably not responsible for. Robert, you cannot repro this even after several tries to update to 3.0.1?
(In reply to comment #14)
> (In reply to comment #12)
> > app.update.log.all boolean value of true
> 
> That's exactly what I've typed in manually after doing a right click and
> selecting "New | Boolean". This happens for a fresh profile. So is another pref
> necessary? Sorry, but I can't get it to work that way. 
bah... the general log everything pref wasn't in 2.0.0.x. I had to add the following to get full logging
user_pref("app.update.log.Checker", true);
user_pref("app.update.log.Downloader", true);
user_pref("app.update.log.General", true);
user_pref("app.update.log.UpdateManager", true);
user_pref("app.update.log.UpdatePatch", true);
user_pref("app.update.log.UpdateService", true);


> (In reply to comment #11)
> > Hmmm... that may be a typo in the 1.8 landing from bug 348389... on 1.9 and
> > trunk that line is licenseAccepted =
> > gUpdates.update.getProperty("licenseAccepted");
> > 
> > Not positive that is the cause but it may be
> 
> I've already changed that, but it still doesn't work. So it's probably not
> responsible for. Robert, you cannot repro this even after several tries to
> update to 3.0.1?
I've just tried to reproduce well over a hundred times on the beta channel without being able to do so. I tried multiple new profiles without and with clearing the cache and performing multiple checks and it worked every single time.

Adding qawanted to see if anyone else is able to reproduce. If you can and you are in the Mountain View office please ping me so I can take a look at your system.
Keywords: qawanted
(In reply to comment #15)
> bah... the general log everything pref wasn't in 2.0.0.x. I had to add the
> following to get full logging

Yeah, that works now. But it looks like that there is no info for my issue. But anyway I'll attach a screenshot with the output messages (copying one by one from the EC because we don't have a multiple selection list :/ is too error-prone).

> I've just tried to reproduce well over a hundred times on the beta channel
> without being able to do so. I tried multiple new profiles without and with
> clearing the cache and performing multiple checks and it worked every single
> time.

Are you running Windows on a native box or also using VmWare? Probably that could be VmWare related? On my Vista VM the problem was gone after installing some system updates and rebooting the System. So probably we only have some timing issues? Even running DOMi (what I already said) I only have to click at the node of the inner document and the text appears. Sadly I cannot offer you my VM. :/ But I could try if I'm able to repro it on my box with Windows installed native.
(In reply to comment #16)
> (In reply to comment #15)
>...
> > I've just tried to reproduce well over a hundred times on the beta channel
> > without being able to do so. I tried multiple new profiles without and with
> > clearing the cache and performing multiple checks and it worked every single
> > time.
> 
> Are you running Windows on a native box or also using VmWare?
Native using Vista.

> Probably that
> could be VmWare related?
Possibly

> On my Vista VM the problem was gone after installing
> some system updates and rebooting the System. So probably we only have some
> timing issues?
Could be. This ui uses a xul browser with everything locked down so it won't be exploited. Could be a problem with its implementation.

> Even running DOMi (what I already said) I only have to click at
> the node of the inner document and the text appears. Sadly I cannot offer you
> my VM. :/ But I could try if I'm able to repro it on my box with Windows
> installed native.
That info would be helpful
(In reply to comment #18)
> > Probably that
> > could be VmWare related?
> Possibly

Now it looks like a combination of VmWare and my MacBook/Mac Mini. It runs fine on my native Windows XP installation.

Both machines (MacBook/Mini) above have the integrated graphics card Intel GMA 950. Someone with access to the Lab could run a test on such a machine? I think that I can't do more atm.
Making severity normal since we no longer show a EULA for Firefox and this appears to affect either some or all installs using a combination of VmWare and MacBook/Mac Mini.

I highly suspect the actual problem is in other code that app update consumes
Severity: critical → normal
Mmh, if we don't show the EULA anymore we don't have a way to test this, right? Probably WONTFIX?
(In reply to comment #21)
> Mmh, if we don't show the EULA anymore we don't have a way to test this, right?
> Probably WONTFIX?

Yes its wontfix, we removed the LicenseURL from the Update Snippets for the Major Updates.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → WONTFIX
By which I assume you mean "perhaps it's wontfix, depending on what other toolkit consumers decide to do about major updates, though at the moment support for a LicenseURL has not yet been removed from the toolkit code"?
(In reply to comment #23)
> By which I assume you mean "perhaps it's wontfix, depending on what other
> toolkit consumers decide to do about major updates, though at the moment
> support for a LicenseURL has not yet been removed from the toolkit code"?

Yes, support for LicenseURL is still there. 

Also as reference the decision for removing the LicenseURL for the Firefox 2.0.0.20->3.0.5 Major Updates is https://bugzilla.mozilla.org/show_bug.cgi?id=470258#c11
Reopening... LicenseURL is used by other app update consumers and there is likely an underlying bug outside of app update that is causing this that it would be a good thing to fix once the underlying cause is identified.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
So do we have any chance to run further tests against any major update? Probably for older builds? Or was the EULA removed for all major updates? It would be hard to find a valid testcase otherwise.
Just as sidenote, when we had the LicenseUrl in place i did a lot of Major Update Testing (about ~ 50 or more) for the Partner Major Update from 2.0.0.18/19 to 3.0.x and i have never seen this problem on any platform (Mac, Windows, Linux), so  there is also. (The Windows Tests were done in a VM Workstation 6.5), so its also still a open question if this is a Problem related to VMware (see comment #19). 

Also this was wfm for me when henrik found this problem and its only confirmed from henrik. Reading the comment of this bug, there is no other confirmation from a 2nd person.
Chances are users won't see this in app update but chances are there is still an underlying bug that could cause problems in other code as was seen by Henrik. There is no harm with leaving a bug like this open and with a bit of luck the underlying problem will be identified... bugs are cheap after all.
I highly suspect there is an underlying core problem when using vmware, etc. per comment #19 and that this just affects app update in a fairly easily noticeable way as has been seen with the migration code. Since we no longer show a license and the problem is likely a core issue resolving -> wontfix
Status: REOPENED → RESOLVED
Closed: 15 years ago14 years ago
Keywords: qawanted
Resolution: --- → WONTFIX
Whiteboard: [MU?]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: