Closed Bug 934103 Opened 11 years ago Closed 10 years ago

"TypeError: aUrl is undefined" when start-up with add-on SDK

Categories

(Firefox :: General, defect)

Product:
Firefox
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 33

People

(Reporter: timdream, Assigned: Gijs)

References

Details

(Whiteboard: [good first bug][lang=js] p=1 s=33.1 [qa-])

Attachments

(2 files, 1 obsolete file)

the result of `hg diff`
(deleted), patch
Details | Diff | Splinter Review
aURL is undefined,
(deleted), patch
dao
: review+
Details | Diff | Splinter Review
+++ This bug was initially created as a clone of Bug #792968 +++ The changes done in bug 792968 did not account the situation if the variable passed into the original regexp.test() actually exists. Instead of changing if (/^xxx/.test(str)) { ... } to if (str.startsWith('xxx')) { ... } it should be be change to if (str && str.startsWith('xxx')) { ... } This has caused "TypeError: aUrl is undefined" error in Firefox whenever Fx is launched with add-on SDK |cfx run| https://hg.mozilla.org/mozilla-central/annotate/396e59370945/browser/base/content/urlbarBindings.xml#l662 It can also occasionally triggered in noermal use (w/o add-on developement), see https://support.mozilla.org/en-US/questions/949594
Could we just prepend undefined check on every line bug 792968 changes? The patch have seem to be causing bug 797430 and bug 845055, and now this.
No longer depends on: 797430, 845055
Attached patch the result of `hg diff` (deleted) — — Splinter Review
Hello. Seems a easy bug to write a few lines of code. I want to work this. But I find that the code I download from http://hg.mozilla.org/mozilla-central/ is still using `if (/^xxx/.test(str))`. For example line 98 in `browser/base/content/browser-feeds.js`. So I should change the code like that to `if (str && str.startsWith('xxx')) { ... }`, right?
Comment on attachment 826395 [details] [diff] [review] the result of `hg diff` Hi mt, Cloud you identify other part of bug 792968 that needs to be fixed too? Not every change need to be prepend with a undefined check though coz some of the variable will always be a string.
Comment on attachment 826405 [details] [diff] [review] bug934103.patch I think mt have this one covered. I believe are many good first bug to solve :)
mt, I will push your patch to Try for you after you submit your second patch.
Assignee: nobody → mt
Attached patch bug934103.patch (obsolete) (deleted) — — Splinter Review
So sorry I forgot to refresh the page. Just ignore my patch.
Attachment #826405 - Attachment is obsolete: true
Comment on attachment 826395 [details] [diff] [review] the result of `hg diff` >--- a/browser/base/content/urlbarBindings.xml Sat Nov 02 16:42:10 2013 -0500 >+++ b/browser/base/content/urlbarBindings.xml Sun Nov 03 13:41:08 2013 +0800 >@@ -659,7 +659,7 @@ > <method name="_parseActionUrl"> > <parameter name="aUrl"/> > <body><![CDATA[ >- if (!aUrl.startsWith("moz-action:")) >+ if (!aUrl || !aUrl.startsWith("moz-action:")) > return null; > > // url is in the format moz-action:ACTION,PARAM aUrl shouldn't be undefined in the first place. Does replacing <field name="_value"></field> with <field name="_value">""</field> in urlbarBindings.xml fix this bug?
Attachment #826395 - Attachment is patch: true
I'm trying to get the auth of Level 1. Hope I can push the patch to try server myself soon.
mt, I've push the WIP patch to try and you will have a Mac build to test out with add-on SDK soon. https://tbpl.mozilla.org/?tree=Try&rev=12a5d0f049ec
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #11) > mt, > > I've push the WIP patch to try and you will have a Mac build to test out > with add-on SDK soon. > > https://tbpl.mozilla.org/?tree=Try&rev=12a5d0f049ec I can confirm mt's patch fix this issue by running this version of Nightly and try it with add-on SDK: cfx run -b /Volumes/Nightly/FirefoxNightly.app/Contents/MacOS/firefox-bin but... (In reply to Dão Gottwald [:dao] from comment #9) > aUrl shouldn't be undefined in the first place. Does replacing <field > name="_value"></field> with <field name="_value">""</field> in > urlbarBindings.xml fix this bug? supposedly I should try this first :-/
> (In reply to Dão Gottwald [:dao] from comment #9) > > aUrl shouldn't be undefined in the first place. Does replacing <field > > name="_value"></field> with <field name="_value">""</field> in > > urlbarBindings.xml fix this bug? > > supposedly I should try this first :-/ Actually, mt, have you figure out how to build Firefox locally? If so you should try it instead.
Flags: needinfo?(mt)
mt, any update? Do you still want to own this bug?
Assignee: mt → nobody
Flags: needinfo?(mt)
Attached patch aURL is undefined, (deleted) — — Splinter Review
Although I hate to take a 'good first bug', this is starting to get really annoying and we should just fix it. I checked, and this does indeed seem to fix the issue at hand.
Attachment #8441347 - Flags: review?(dao)
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Attachment #8441347 - Flags: review?(dao) → review+
Mentor: dao
Whiteboard: [good first bug][mentor=dao][lang=js] → [good first bug][lang=js]
remote: https://hg.mozilla.org/integration/fx-team/rev/8e653defe8b5 Marco, can you add this to 33.1 ?
Flags: needinfo?(mmucci)
Whiteboard: [good first bug][lang=js] → [fixed-in-fx-team][good first bug][lang=js] p=1 s=33.1 [qa-]
Added to Iteration 33.1
Flags: needinfo?(mmucci)
https://hg.mozilla.org/mozilla-central/rev/8e653defe8b5
Mentor: dao
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team][good first bug][lang=js] p=1 s=33.1 [qa-] → [good first bug][lang=js] p=1 s=33.1 [qa-]
Target Milestone: --- → Firefox 33
Mentor: dao
Flags: firefox-backlog+
Mentor: dao
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: