[review-request] [PATCH] dreyna/dir_expand_7810

[review-request] [PATCH] dreyna/dir_expand_7810

[Reyna, David]

Hi Belén,

I have pushed a fix for 7810:
   dreyna/dir_expand_7810

It is very strange. The problem was that the AJAX call in "dirinfo.html" expects the response data to be a JSON-formatted string, and uses "parseJSON()" to convert that into a JS object for display computation.

However, in debugging the code I found that the JS debugger recognized the "response" to now _already_ be a proper JS object, and the observed failure in the defect is that "parseJSON()" tries to convert it and immediately hits a syntax error. 

The fix is to simply skip the "parseJSON()", and now it all works. But I do not know why the AJAX operation changed, unless it was updated to take the JSON format indicator and perform the conversion automatically now, which would explain why it works the way it does.

In any case, I have kept a note in the the patch indicated how "parseJSON()" is no longer needed, in case someone in the future puzzles over this.

- David

Re: [review-request] [PATCH] dreyna/dir_expand_7810

[Michael Wood]

On 19/08/15 08:34, Reyna, David wrote:
> Hi Belén,
>
> I have pushed a fix for 7810:
>     dreyna/dir_expand_7810
>
> It is very strange. The problem was that the AJAX call in "dirinfo.html" expects the response data to be a JSON-formatted string, and uses "parseJSON()" to convert that into a JS object for display computation.
>
> However, in debugging the code I found that the JS debugger recognized the "response" to now _already_ be a proper JS object, and the observed failure in the defect is that "parseJSON()" tries to convert it and immediately hits a syntax error.
>
> The fix is to simply skip the "parseJSON()", and now it all works. But I do not know why the AJAX operation changed, unless it was updated to take the JSON format indicator and perform the conversion automatically now, which would explain why it works the way it does.
>
> In any case, I have kept a note in the the patch indicated how "parseJSON()" is no longer needed, in case someone in the future puzzles over this.
>
> - David
>

Yes you're right, when an ajax call gets a JSON response it's 
automatically parsed to a js object. Previously the response it was 
getting was just a text/other response and therefore the manual parse 
was needed. In commit:

commit c5a16235b8b56102703bc264768ba386ebe90611
Author: Alexandru DAMIAN <alexandru.damian@intel.com>
Date:   Mon May 11 18:58:25 2015 +0100

     bitbake: toaster: fix html5 compliance

     This patch brings needed changes in all views so that
     each view passes the HTML5 compliance test by the
     W3C Markup Service.


This was changed:

-    return HttpResponse(_get_dir_entries(build_id, target_id, top))
+    return HttpResponse(_get_dir_entries(build_id, target_id, top), 
content_type = "application/json")

So the response type is now (correctly) marked as json and the ajax call 
does the automatic parsing.

I don't think there is a need for the " /* object ready, no need for 
$.parseJSON(response); */" comment as this is the expected behaviour. So 
I’ll submit the patch without this if that's OK.

Thanks,

Michael

Re: [review-request] [PATCH] dreyna/dir_expand_7810

[Reyna, David]

>  So I’ll submit the patch without this if that's OK.

Yes, that would be great!

David
> On Aug 19, 2015, at 4:25 AM, Michael Wood <michael.g.wood@intel.com> wrote:
> 
> So I’ll submit the patch without this if that's OK.