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

VERIFIED FIXED in Firefox 53

Status

()

Firefox
Preferences
P5
normal
VERIFIED FIXED
6 months ago
3 months ago

People

(Reporter: Paenglab, Assigned: Alan, Mentored)

Tracking

unspecified
Firefox 53
Points:
---

Firefox Tracking Flags

(firefox53 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

6 months 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

6 months 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@gmail.com
Priority: -- → P5
Summary: ReferenceError: reference to undefined property this._openedURL → Strict warning: ReferenceError: reference to undefined property this._openedURL
(Assignee)

Comment 2

3 months 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

3 months 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

3 months 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

3 months 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

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

Comment 7

3 months 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

3 months ago
Created attachment 8827298 [details] [diff] [review]
bug

Comment 9

3 months 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

3 months 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

3 months 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')
Comment hidden (mozreview-request)
(Assignee)

Comment 13

3 months 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

3 months 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

3 months ago
Attachment #8827699 - Flags: review?(gijskruitbosch+bugs)
(Assignee)

Comment 15

3 months 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

3 months ago
Flags: needinfo?(ayx002)

Comment 16

3 months 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

3 months 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

3 months 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

3 months 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

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

Comment 21

3 months 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 22

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/92ac20f11d85

Comment 23

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

Comment 24

3 months 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.