166448 - Software Update Notification dialog never supposed to show, but does periodically

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect, P1)

Description

[Samir Gehani]

Summary:
The Software Update Notification dialog is never supposed to show up on MacOS
X. Instead it shows up every time we check for an update (~1week), reporting
that an update is available even though one is not. This symptom only affects
Netscape builds although the code is in the mozilla tree (work in progress for
mozilla).

To repro:
Just wait 7-13 days or follow a complicated and detailed set of steps that I
won't list here.

Actual results:
approx. every week a dialog will popup informing users that a browser update is
available and offering a button to click that will take them to download the
newest version(and it will).

Expected results:
Absolutely nothing. Updates should fail silently on MacOSX.

Platform info:
MacOS X specific. branch and logically the trunk. tested with 2002082305 branch
build (rtm candidate).

Original report:
<http://bugscape/show_bug.cgi?id=19560> by Claudius Gayle.

Comment 1

[Samir Gehani]

Attachment: Brute force approach.

Check is we are on Mac OS X and don't register the update notifier component if
so.

Comment 2

[Samir Gehani]

Attachment: Fail silently if old installation file not found.

Comment 3

[Daniel Veditz [:dveditz]]

Comment on attachment 98062 [details] [diff] [review]
Fail silently if old installation file not found.

sr=dveditz

Comment 4

[Bill Law]

Comment on attachment 98062 [details] [diff] [review]
Fail silently if old installation file not found.

r=law

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 98062 [details] [diff] [review]
Fail silently if old installation file not found.

a=[e-mail]

Comment 6

[Samir Gehani]

Fix checked into trunk.

Comment 7

[Michael Buckland]

Making blocker for Blackbird meta.  Please hold checkin until you get the go
ahead to avoid probs with other branch dwellers.

Comment 8

[Daniel Veditz [:dveditz]]

Please add the adt1.0.2 keyword when a branch patch is ready

Comment 9

[Michael Buckland]

Samir, I believe you have a branch patch ready.  Can you please check it in, or
else let me know it's not ready?

Comment 10

[Paul Wyskoczka]

Changing QA contact to pmac

Comment 11

[Samir Gehani]

Reopening so that we can make this more robust: use Necko if we are suspicious
of XPInstall results.

Comment 12

[Michael Buckland]

samir, there is not time to make this more robust.  Dan can you review the
original patch and weigh in on whether we can use that or need to think about
dropping the feature?

Comment 13

[Samir Gehani]

Attachment: Use version from necko if we are updating the browser (i.e., registryName == ``Browser'').

Comment 14

[Samir Gehani]

Attachment: Use neck version, take 2.

Same patch as above modulo dump statements removed.

Comment 15

[Michael Buckland]

Added adt1.0.2.  Dan can you please review this patch?  Samir, thanks!

Comment 16

[Samir Gehani]

Attachment: Fix version parsing.

We now parse the vendorSub portion of the version accounting for leading
zeroes, only zeroes, and no leading zeroes.  Thanks to Dan for pointing this
out.

   x.0	 =>   x.0.0
   x.0y  =>   x.0.y
   x.y	 =>   x.y.0
   x.y0  =>   x.y0.0

Comment 17

[Bill Law]

Comment on attachment 101337 [details] [diff] [review]
Fix version parsing.

r=law

This is extremely well-written code (or at least easy-to-understand code).  You
did misspell "contrcutor" but personally I don't get to worked up about such
things.

Comment 18

[Samir Gehani]

Thanks for the review Bill.  Spelling error corrected in my local patch.

Comment 19

[Daniel Veditz [:dveditz]]

er, this is a Mozilla bug and Mozilla doesn't have a vendorSub. Should you add a
backup plan, like try to parse the rv: version if no vendorSub? Other vendors
might use versions that look like "X.0.Y", or less likely even just "X" with no
dots or something like a.b.c.d -- would your code handle those cases?

Comment 20

[Samir Gehani]

My code will handle the x-only case by just using x and appending "." +
productSub yielding x.productSub.

The x.0.y case becomes hairy as does a.b.c.d because they simply come back with
a .0 tacked on yielding a.b.c.d.0, for example.

Having said that, I agree this approach is not general.  In fact it's brittle
nature makes me nervous.  It is very specific.  Maybe we need to find the reason
why we are getting the NOT_FOUND error from the InstallTrigger code and continue
to rely on that?  Then I won't have to make these changes.  

Alternatively, I could morph the format of the updates.rdf entirely to eliminate
registryName and use vendorSub and productSub and just do comparisons on those.
 Looking for any suggestions on a comprehensive, general solution.

Comment 21

[Samir Gehani]

Attachment: Use necko version, take 3.

Synthesize version from misc's rv: value and the productSub.  Upon discussion
with dveditz we arrived at this solution since it will be usable by any Gecko
vendor/embeddor.

Comment 22

[Daniel Veditz [:dveditz]]

Comment on attachment 102038 [details] [diff] [review]
Use necko version, take 3.

>+          synthesized += ((len >= i+1) ? parts[i] : "0") + ".";

nit: this would be more efficient as (len > i) to avoid extra calculation in
the loop. Not a big deal since this loop only goes 3 times through but things
like this can make a huge difference in JS performance for larger loops.

sr=dveditz

Comment 23

[harishd]

Comment on attachment 102038 [details] [diff] [review]
Use necko version, take 3.

>+    if (onlyVer && onlyVer.length >= 2)

Add a comment why you're checking for onlyVer's length to be >=2.

With that r=harishd

Comment 24

[Daniel Veditz [:dveditz]]

Please land on the 1.0 branch and add the fixed1.0.2 keyword. Since this is not
shipped in Mozilla builds (yet) I'm going to guess you don't actually need a=
for the branch.

Comment 25

[Samir Gehani]

Fix checked in on branch and trunk.

Comment 26

[Patty Mac]

I checked on windows 2000, linux redhat 7.3, mac os 9.2.2,netscape branch build: 
2002-10-09-05-1.0)

Step #1: (client side)
(1) Install the 2002-10-09-05-1.0
(2) Go to C:\Builds\1009080510\chrome\US.jar
(3) On windows, use winzip to extract the files.
(4) The "LOCAL" folder is already unzipped.
(5) Go to C:\Builds\files\locale\US\communicator-region
(6) Open 'region.properties".
(7) Updated _notifications.provider.0.datasource with your url (eg:
update_notifications.provider.0.datasource=http://jimbob/updates.rdf
(8)Save it.
(9) Move the system clock 'backward' 4 weeks.
(10)Shutdown the client.
(11)Then start the client again
(12)There is no notification dialog, it's correct behavior.

Step #2:

1) Install the 2002-10-09-05-1.0
(2) Go to C:\Builds\1009080510\chrome\US.jar
(3) On windows, use winzip to extract the files.
(4) The "LOCAL" folder is already unzipped.
(5) Go to C:\Builds\files\locale\US\communicator-region
(6) Open 'region.properties".
(7) Updated _notifications.provider.0.datasource with your url (eg:
update_notifications.provider.0.datasource=http://jimbob/updates.rdf
(8) save it.
(9) Move the system clock 'forward' 4 weeks.
(10)Shutdown the client.
(11)Then start the client again
(12)I wait even more than 5 minutes, There is no notification dialog popped up.
The notification dialog should pop up.

Server side:

(1) Go to http://jimbob/updates.rdf and changed the version number only:
(e.g: the build id # is: 1.0.2.20021009) or you can go to this server to get it
http://www.mozilla.org/xpapps/updates/updates.rdf.txt

Here is the rdf file that I already modified the version number:
<?xml version="1.0"?>
<RDF:RDF xmlns:RDF="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
         xmlns:NC="http://home.netscape.com/NC-rdf#">

          <RDF:Description about="urn:updates:latest">
    <NC:registryName>Browser</NC:registryName>
    <NC:version>1.0.2.20021009</NC:version>
    <NC:URL>http://mozilla.org/releases</NC:URL>
    <NC:productName>Mozilla</NC:productName>
  </RDF:Description>

</RDF:RDF>

(2) Save it as updates.rdf 

Note: I even checked the pref.js and it showed
user_pref("update_notifications.provider.0.last_checked", 1035351257);

Samir, is this bug fixed or perhaps something wrong with my setups?
Thanks!

Comment 27

[Samir Gehani]

Added text/rdf MIME type to the server Patty was using to test.  

Comment 28

[Patty Mac]

Verified on all platforms (netscape branch build: 2002-10-09-08-1.0)