Strict warning: ReferenceError: reference to undefined property this._openedURL

VERIFIED FIXED in Firefox 53

Status

()

P5
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: Paenglab, Assigned: ayx002, Mentored)

Tracking

unspecified
Firefox 53
Points:
---

Firefox Tracking Flags

(firefox53 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
When I open a subdialog I get this error:
19:58:19.849 ReferenceError: reference to undefined property this._openedURL 1 subdialogs.js:46:1

Comment 1

2 years ago
That's a strict warning, despite the "ReferenceError" warning, I'm pretty sure. Everything is working correctly.

We could explicitly set the value to null and reset to null instead of calling delete;. That would silence this warning.
Mentor: gijskruitbosch+bugs
Priority: -- → P5
Summary: ReferenceError: reference to undefined property this._openedURL → Strict warning: ReferenceError: reference to undefined property this._openedURL
(Assignee)

Comment 2

2 years ago
(In reply to :Gijs from comment #1)
> That's a strict warning, despite the "ReferenceError" warning, I'm pretty
> sure. Everything is working correctly.
> 
> We could explicitly set the value to null and reset to null instead of
> calling delete;. That would silence this warning.

I assume from what you have said this is the line causing issues:
https://dxr.mozilla.org/mozilla-central/source/browser/components/preferences/in-content/subdialogs.js#115

From what I read online and saw in the code the issue is caused because we are trying to delete this._openedURL and if it is ever used again it causes the error?

A fix would be to:
Set this._openedURL to null instead of calling delete.
(Assignee)

Comment 3

2 years ago
(In reply to Richard Marti (:Paenglab) from comment #0)
> When I open a subdialog I get this error:
> 19:58:19.849 ReferenceError: reference to undefined property this._openedURL
> 1 subdialogs.js:46:1

What steps did you take to produce this error?

Comment 4

2 years ago
(In reply to Alan from comment #2)
> (In reply to :Gijs from comment #1)
> > That's a strict warning, despite the "ReferenceError" warning, I'm pretty
> > sure. Everything is working correctly.
> > 
> > We could explicitly set the value to null and reset to null instead of
> > calling delete;. That would silence this warning.
> 
> I assume from what you have said this is the line causing issues:
> https://dxr.mozilla.org/mozilla-central/source/browser/components/
> preferences/in-content/subdialogs.js#115

Yes. Using this._openedURL = null; instead would fix this.

> From what I read online and saw in the code the issue is caused because we
> are trying to delete this._openedURL and if it is ever used again it causes
> the error?

Yeah, on debug builds the JS engine logs warnings when you rely on a property's value if that property does not exist. We delete the property, but if we set it to null instead that will shut up the JS engine's warnings (because the property will always exist - it'll just be null some of the time.

(In reply to Alan from comment #3)
> (In reply to Richard Marti (:Paenglab) from comment #0)
> > When I open a subdialog I get this error:
> > 19:58:19.849 ReferenceError: reference to undefined property this._openedURL
> > 1 subdialogs.js:46:1
> 
> What steps did you take to produce this error?

I expect you need a debug build, and then open the preferences/options, and e.g. go to the "content" section and click a button that shows a dialog, like the "Exceptions..." one next to popups. You might need to close and reopen to see this error, I'm not sure off-hand.
(Reporter)

Comment 5

2 years ago
Open FX, then the prefs.
Open and clear JS console.
Go to Content tab and open a subdialog (e. g. Colors).
Error appears only the first time. You have to close the prefs to see the error again on opening a dialog. For me it appears on every sub dialog.

But like Gijs wrote, you have to set javascript.options.strict to true to see the error.
(Reporter)

Comment 6

2 years ago
No debug build needed (I never use one). Only set javascript.options.strict to true.
(Assignee)

Comment 7

2 years ago
So I made the change to the subdialogs.js file, however, I have a lot of @TOPSCRDIR@ files that I committed. Now when I "hg push review" the searching for changes part is taking hours so I have been able to hg push review correctly. Can someone explain what I did wrong?
(Assignee)

Comment 8

2 years ago
Posted patch bugSplinter Review

Comment 9

2 years ago
(In reply to Alan from comment #7)
> So I made the change to the subdialogs.js file, however, I have a lot of
> @TOPSCRDIR@ files that I committed. Now when I "hg push review" the
> searching for changes part is taking hours so I have been able to hg push
> review correctly. Can someone explain what I did wrong?

Hey Alan, what specifically do you mean when you say you have a lot of committed @TOPSRCDIR@ files? If you want to remove those commits, you can likely pass the revision of the broken commit (with -r ) to either 'hg prune' or 'hg strip' to do so (though be careful as this permanently removes the revisions in question from your local repository).
Flags: needinfo?(ayx002)
(Assignee)

Comment 10

2 years ago
(In reply to :Gijs from comment #9)
> (In reply to Alan from comment #7)
> > So I made the change to the subdialogs.js file, however, I have a lot of
> > @TOPSCRDIR@ files that I committed. Now when I "hg push review" the
> > searching for changes part is taking hours so I have been able to hg push
> > review correctly. Can someone explain what I did wrong?
> 
> Hey Alan, what specifically do you mean when you say you have a lot of
> committed @TOPSRCDIR@ files? If you want to remove those commits, you can
> likely pass the revision of the broken commit (with -r ) to either 'hg
> prune' or 'hg strip' to do so (though be careful as this permanently removes
> the revisions in question from your local repository).

I miss spoke about the TOPSRCDIR files. But when I am trying to "hg push review" it's taking ~2 hours. I am thinking about cloning the repo again. What do you think is the best? Yeah I read about strip and prune.

Comment 11

2 years ago
(In reply to Alan from comment #10)
> (In reply to :Gijs from comment #9)
> > (In reply to Alan from comment #7)
> > > So I made the change to the subdialogs.js file, however, I have a lot of
> > > @TOPSCRDIR@ files that I committed. Now when I "hg push review" the
> > > searching for changes part is taking hours so I have been able to hg push
> > > review correctly. Can someone explain what I did wrong?
> > 
> > Hey Alan, what specifically do you mean when you say you have a lot of
> > committed @TOPSRCDIR@ files? If you want to remove those commits, you can
> > likely pass the revision of the broken commit (with -r ) to either 'hg
> > prune' or 'hg strip' to do so (though be careful as this permanently removes
> > the revisions in question from your local repository).
> 
> I miss spoke about the TOPSRCDIR files. But when I am trying to "hg push
> review" it's taking ~2 hours. I am thinking about cloning the repo again.
> What do you think is the best? Yeah I read about strip and prune.

I don't know why that would happen. Are you passing a revision to push (so something like "hg push -r . review"), and if not, does passing a rev go any faster?

If that doesn't help, you can also use "hg export" to create a patch file and just attach it to the bug. It looks like you already did that, but the commit you attached doesn't have the actual change, but seems to be on top of the earlier change (from 'delete this._openedURL' to 'this._openedURL = null')
(Assignee)

Comment 13

2 years ago
(In reply to :Gijs from comment #11)
> (In reply to Alan from comment #10)
> > (In reply to :Gijs from comment #9)
> > > (In reply to Alan from comment #7)
> > > > So I made the change to the subdialogs.js file, however, I have a lot of
> > > > @TOPSCRDIR@ files that I committed. Now when I "hg push review" the
> > > > searching for changes part is taking hours so I have been able to hg push
> > > > review correctly. Can someone explain what I did wrong?
> > > 
> > > Hey Alan, what specifically do you mean when you say you have a lot of
> > > committed @TOPSRCDIR@ files? If you want to remove those commits, you can
> > > likely pass the revision of the broken commit (with -r ) to either 'hg
> > > prune' or 'hg strip' to do so (though be careful as this permanently removes
> > > the revisions in question from your local repository).
> > 
> > I miss spoke about the TOPSRCDIR files. But when I am trying to "hg push
> > review" it's taking ~2 hours. I am thinking about cloning the repo again.
> > What do you think is the best? Yeah I read about strip and prune.
> 
> I don't know why that would happen. Are you passing a revision to push (so
> something like "hg push -r . review"), and if not, does passing a rev go any
> faster?
> 
> If that doesn't help, you can also use "hg export" to create a patch file
> and just attach it to the bug. It looks like you already did that, but the
> commit you attached doesn't have the actual change, but seems to be on top
> of the earlier change (from 'delete this._openedURL' to 'this._openedURL =
> null')

Finally was able to get my repo working! 'hg roots(outgoing())' got rid of my outgoing changes and I did a hg update --clean. Thanks for the help with everything Gijs I really appreciate it.
Flags: needinfo?(ayx002)

Comment 14

2 years ago
Great! So now, we need to get someone to review it. If you make the commit message end in ", r?gijs" and re-push it (you can use "hg commit --amend" to adjust the last commit, and then just use 'hg push -r . review' again), it'll automatically ask me for review.

(I would have set it in reviewboard myself, but it's telling me you have a draft in addition to the now public changeset, and so it won't let me change it while there's a new unpublished draft revision...)
Assignee: nobody → ayx002
Status: NEW → ASSIGNED
Flags: needinfo?(ayx002)
(Assignee)

Updated

2 years ago
Attachment #8827699 - Flags: review?(gijskruitbosch+bugs)
(Assignee)

Comment 15

2 years ago
(In reply to :Gijs from comment #14)
> Great! So now, we need to get someone to review it. If you make the commit
> message end in ", r?gijs" and re-push it (you can use "hg commit --amend" to
> adjust the last commit, and then just use 'hg push -r . review' again),
> it'll automatically ask me for review.
> 
> (I would have set it in reviewboard myself, but it's telling me you have a
> draft in addition to the now public changeset, and so it won't let me change
> it while there's a new unpublished draft revision...)

I think I forgot to publish haha.
(Assignee)

Updated

2 years ago
Flags: needinfo?(ayx002)

Comment 16

2 years ago
mozreview-review
Comment on attachment 8827699 [details]
Bug 1312216 - Changed delete this._openedURL and set it as null instead. There was a strict warning.

https://reviewboard.mozilla.org/r/105324/#review106454
Attachment #8827699 - Flags: review?(gijskruitbosch+bugs) → review+

Comment 17

2 years ago
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/580064ea1d81
Changed delete this._openedURL and set it as null instead. There was a strict warning. r=Gijs

Comment 18

2 years ago
(In reply to Alan from comment #15)
> (In reply to :Gijs from comment #14)
> > Great! So now, we need to get someone to review it. If you make the commit
> > message end in ", r?gijs" and re-push it (you can use "hg commit --amend" to
> > adjust the last commit, and then just use 'hg push -r . review' again),
> > it'll automatically ask me for review.
> > 
> > (I would have set it in reviewboard myself, but it's telling me you have a
> > draft in addition to the now public changeset, and so it won't let me change
> > it while there's a new unpublished draft revision...)
> 
> I think I forgot to publish haha.

Fabulous, this works - thanks!
(Reporter)

Comment 19

2 years ago
I still get the error with the patch applied.

Shouldn't it be a _openedURL: null, after var gSubDialog = { to define _openedURL initially?

Comment 20

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/580064ea1d81
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53

Comment 21

2 years ago
(In reply to Richard Marti (:Paenglab) from comment #19)
> I still get the error with the patch applied.
> 
> Shouldn't it be a _openedURL: null, after var gSubDialog = { to define
> _openedURL initially?

D'oh, I should have caught this in review. Followup:

https://hg.mozilla.org/integration/mozilla-inbound/rev/92ac20f11d85

Comment 23

2 years ago
Richard, can you confirm this is fixed now?
Flags: needinfo?(richard.marti)
(Reporter)

Comment 24

2 years ago
Yes, this is fixed now.
Thank you Alan and Gijs.
Status: RESOLVED → VERIFIED
Flags: needinfo?(richard.marti)
You need to log in before you can comment on or make changes to this bug.