Javascript web service client parses HTML representations as entries, even if they're not

Bug #380959 reported by Leonard Richardson on 2009-05-27
14
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Launchpad itself
High
Leonard Richardson

Bug Description

Edwin wrote a function called parse_xml_entry which tears apart a default XHTML representation of an entry and makes a Javascript object out of it, the same way that the web service client currently turns a JSON representation of an entry into a Javascript object.

The problem is that the default XHTML representation is just that--a default. It's not necessarily a <dl> tag with a bunch of <dt> tags underneath it. It could be any HTML at all. And even if it is a <dl> tag, it doesn't make sense to tear it apart. If you need an Entry object, you can just request a JSON representation. XHTML representations are intended for display as part of web pages. So parse_xml_entry shouldn't even exist. The response, or its responseText, or responseXML (I'm not sure which) should be passed directly into the callback method, which is responsible for integrating the HTML data into the web page.

But, I'm guessing Edwin didn't write this code for no reason. So I'm filing this bug so that we can work it out. We need to 1) figure out how to do what Edwin wants to do, 2) figure out whether we should be passing the callback the response object itself, response.responseText, or response.responseXML, and 3) get rid of parse_xml_entry.

Leonard Richardson (leonardr) wrote :

I've associated a branch with this bug which changes wrap_resource_on_success. When the client requests an XHTML representation, it passes response.responseText into the callback method instead of calling parse_xml_entry. This is a branch that Edwin can hack on (to make his code call parse_xml_entry itself) and Bjorn can use to get his work done.

In IRC Edwin explained that he created parse_xml_entry because he was sending a PATCH request that modified a single field, and he needed the HTML representation of that specific field. This is exactly the use case for bug 369887, which will let you PATCH a single field and get a representation of just that field. Once I fix 369887, we can get rid of parse_xml_entry.

Björn Tillenius (bjornt) wrote :

Leonard, it seems like you forgot to push your changes, that branch doesn't contain any new revisions.

Leonard Richardson (leonardr) wrote :

Sorry, I've pushed it now.

Edwin Grubbs (edwin-grubbs) wrote :

I added the explicit call to LP.client.parse_xml_entry() in the branch below. This code isn't actually used anywhere yet, since Tom Berger is integrating multiple bug page changes for landing at the same time.

lp:~edwin-grubbs/launchpad/bug-380959-parsing-xml-entry

Björn Tillenius (bjornt) wrote :

Edwin, what's left to do before you can land that branch? I have a branch almost ready for landing that is blocking on this.

Francis J. Lacoste (flacoste) wrote :

This change also broke 3 tests in the client.js Windmill suite.

(It expected responseText to be passed in) which is the proper thing to revert to.

Now, that we have PUT support for fields, I think we can remove this code (and make sure that the windmill/jstests suite is clean again).

Changed in launchpad:
importance: Undecided → High
status: New → Triaged
affects: launchpad → launchpad-foundations
Changed in launchpad-foundations:
assignee: nobody → Leonard Richardson (leonardr)
Edwin Grubbs (edwin-grubbs) wrote :

The changes I made to my side to make sure that this doesn't break the inline bug assignee picker has been added in the branch above. I assumed that Leonard would be landing this since it is assigned to him.

Soory you are at the wrong email and person Idon't why I'm not a
professionnel in fixing and reapirind Bugs in Ubuntu I wish you find a
solution good luck.

Leonard Richardson (leonardr) wrote :

Edwin, not assigning it to you was my oversight. However I am now responsible for getting rid of parse_xml_entry, so it's my bug now.

Leonard Richardson (leonardr) wrote :

I dropped the ball on this bug. BjornT needs the code, so I'm giving the branch to him along with testing instructions. BjornT, feel free to give the bug back to me and I'll address it when I have time.

Changed in launchpad-foundations:
assignee: Leonard Richardson (leonardr) → Björn Tillenius (bjornt)
Leonard Richardson (leonardr) wrote :

This branch fixed some test failures, and didn't break any that weren't already broken, so I went ahead and landed it.

Changed in launchpad-foundations:
status: Triaged → Fix Committed
assignee: Björn Tillenius (bjornt) → Leonard Richardson (leonardr)
Curtis Hovey (sinzui) on 2009-10-18
Changed in launchpad-foundations:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers