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`
475 bytes, patch
Details | Diff | Splinter Review
aURL is undefined,
1.43 KB, 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

    
    

    
  
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)
        — 
          — 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)

    
    

    
  


  
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: