158167 - Form Manager loses data ("Access to restricted URI denied")

Status: VERIFIED FIXED

(Toolkit :: Form Manager, defect)

Description

[Greg K.]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:1.1b) Gecko/20020718
BuildID:    2002071808

The Form Manager fails to store entered data. After attempting to re-enter the
lost data, clicking the "OK" button on the Form Manager window crashes Mozilla.

Reproducible: Always
Steps to Reproduce:
1. Summon the Form Manager window
2. Enter your first and last name
3. Click OK; notice nothing happens
4. Click OK again; notice window dismisses
5. Summon Form Manager window again; notice previously entered data is missing
6. Re-enter first and last name
7. Click OK

Actual Results:  Mozilla loses the entered data and crashes.

Expected Results:  Mozilla should have stored the entered data from step 2 the
first time, and should not have crashed on subsequent re-entry attempt.

Crash report forthcoming.

Comment 1

[Greg K.]

Attachment: (stack for crash @nsXULElement::GetOwnerDocument - Bug #158202)

Comment 2

[Greg K.]

Note: I duplicated this using a new Mozilla user profile.

Comment 3

[Terri Preston]

okay, this works just fine for me, using mac OS X branch build 2002-07-18-05,
reporter, when you say summon the form manager window, do you mean From the
Tools Drop Down menu, select Form Manager ->Edit Form info????

Comment 4

[Greg K.]

Yes; Tools/Form Manager/Edit Form Info.

Comment 5

[Tracy Walker [:tracy]]

I am seeing this on trunk builds today on all platforms.

-goto Tools | Form Manager | Edit Form Info
-enter a first and last name
-click "okay"
-go back to "Edit Form Info"
the data you just entered is gone.
-attempt reenter the names then click okay....crash

Comment 6

[Tracy Walker [:tracy]]

odd thing is...if you do a Edit | Fill in form,  the data is there and can be
filled into a form...otherwise this would be a blocker.

Comment 7

[Terri Preston]

Aha, it's trunk only, I see this as well with Trunk build 2002071808

Comment 8

[Stephen P. Morse]

I see something similar with today's trunk build as well.  I enter the data, 
close the dialog, return to the dialog, and the data is not there.

But I am not observing a crash.  I reentered the data a second time, clicked ok, 
and the dialog closed the second time.  Of course when I went back to the dialog 
for the third time, it was still blank.

Also I did not have to press OK twice to get the dialog to close (step 3 in the 
description) -- it closed on the first click.

This is on winnt.

Comment 9

[Tracy Walker [:tracy]]

this is a blocker on OSX.  the field data is not remembered at all.  

-after filling in names in edit form info go to Edit | Fill In Form
you can't because the data isn't there to make the selection available. (the
selection is greyed out)

Comment 10

[Stephen P. Morse]

Mitch, did you change something in the security module on the trunk in the last 
few days that could account for this.  The javascript console is giving the 
following messages:

Security Error: Content at 
   chrome://communicator-region/locale/wallet/WalletName.xul
   may not load data from
   chrome://communicator/content/wallet/WalletViewer.xul.

Error: uncaught exception: [Exception... "Access to restricted URI denied"  
   code: "1012" nsresult: "0x805303f4 (NS_ERROR_DOM_BAD_URI)"
   location: "chrome://communicator/content/wallet/WalletViewer.js Line: 243"]

To reproduce I did the following:
1. Create fresh profile
2. open form-manager dialog with TOOLS -> FORM-MANAGER -> EDIT-FORM-INFO
3. type some characters into any field
4. click OK and form-manager dialog closes
5. again open form-manager dialog with TOOLS -> FORM-MANAGER -> EDIT-FORM-INFO

Error messages appear after step 5

Comment 11

[Stephen P. Morse]

Error is occuring on the following line in walletviewer.js

              for (var j = 0; j<strings.length-1; j++) {
                var menuItem = ...
                 if (menuItem) {
                  menuItem.setAttribute("label", strings[j]);
                  menuItem.setAttribute("len", strings[j].length);
 >>>>>>           menuPopup.insertBefore(menuItem, menuPopup.lastChild);
                }

This code executes when WalletName.xul loads.  The sequence is as follows:

   WalletName.xul from locale/US/communicator-region/wallet/:
      onload="parent.initPanel();"
   walletviewer.js from content/communicator/wallet/:
      function initPanel() {
         hWalletViewer.onpageload(tag)
      }
      ...
      onpageload:
      ...
         menuPopup.insertBefore(menuItem, menuPopup.lastChild);

Comment 12

[timeless]

the stack trace is bug 158202 - there's a patch w/ r+sr+a waiting for a
sheriff/buildmonkey to approve in it. sicking is responisble for messing with
security recently :)

Comment 13

[David G King]

As per Comment #12, should this be marked as a dup of Bug #158202?

Comment 14

[Greg K.]

No. If that covers the crash, this bug should remain to cover the dataloss.

Comment 15

[David G King]

That's true, however if it fixes both the crash and the dataloss, then it should
be a dup. I hope it fixes both problems, but maybe I'm just being an optimist ;-)

Comment 16

[Tracy Walker [:tracy]]

still have data loss in form manager with respins:

windows 2002-07-19-11-trunk
linux 2002-07-19-11-trunk
max osx 2002-07-19-11-trunk

However, the builds are no longer crashing when trying to reenter data in the
form manager, just not remembering what was input.

crashing was not what made this a blocker, ugly, but the data loss is the
blocker cause.

Comment 17

[Stephen P. Morse]

Let me repeat.  The dataloss is being caused by the security violation described 
in comment 10.  Here's hoping that mitch knows what changed in this area 
recently.

I'm still investigating.

Comment 18

[Mitchell Stoltz (not reading bugmail)]

This may have been caused by sicking's security changes, bug 156452.

Comment 19

[Mitchell Stoltz (not reading bugmail)]

We recommend this be downgraded from blocker. jst is working on the problem and
opening the tree will not make the fix any more difficult.

Comment 20

[Samir Gehani]

Downgrading severity per recommendation.

Comment 21

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: patch to fix

Allows "chrome-nodes" to do anything.

Comment 22

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

ccing for reviews

Comment 23

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 92042 [details] [diff] [review]
patch to fix

r=bzbarsky.  On trunk we should consider pushing that logic down into the
security manager.....

Comment 24

[Johnny Stenback (:jst)]

Comment on attachment 92042 [details] [diff] [review]
patch to fix

sr=jst

Comment 25

[Asa Dotzler [:asa]]

Comment on attachment 92042 [details] [diff] [review]
patch to fix

a=asa (on behalf of drivers) for checkin to 1.1

Comment 26

[Brant Gurganus]

Bug #158432 may be related to or a duplicate of this bug.

Comment 27

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

err.. should have taken this before

Comment 28

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

checked in

Comment 29

[lchiang]

tpreston - pls verify this on the trunk and to be safe, on the branch also,
since the fix which caused this regression is also planned for the branch.

Comment 30

[Tracy Walker [:tracy]]

this works fine with trunk builds from 0722-0723 and on branch builds from 0722

marking verified

Comment 31

[Terri Preston]

thanks tracy