crschmidt (![[personal profile]](https://www.dreamwidth.org/img/silk/identity/user.png)
crschmidt) wrote in ![[site community profile]](https://www.dreamwidth.org/img/comm_staff.png)
dw_dev_training2012-05-05 09:01 am
crschmidt) wrote in dw_dev_training2012-05-05 09:01 am
Entry tags:
Working on a DW Bug: Step by Step
So, when I woke up this morning, in #dreamwidth, there was a discussion of a bug in auto-expanding lj-cuts in a particular entry. It looked interesting enough that I figured I would poke at it, since there was a theory it was a Javascript bug, and Javascript is a language I speak. (The actual bug was that clicking on a particular lj-cut expansion would expand to a different lj-cut text, now filed as Bug 4435.)
First, I didn't even know how to expand LJ-cuts inline. So I started from pretty much nothing here. :) But in the end, I think I found the bug, and attached a patch to Zilla, so I figured I'd write up my process.
Start: (6:46) Okay, clicking on the cut gives the wrong thing. Open up my browser's web console, and check the HTTP request that it's sending is right -- it specifies cutid=1, but is given one from a much later cut. (6:48) To me, this suggests that the problem is server side (not Javascript), so I have to dive into code I can't just load in my browser. I experiment a bit more to ensure that this appears to be the case, and determine I can't see any evidence that this is client side. (6:58).
I Googled 'dreamwidth code' (which didn't get me very far), then 'dreamwidth mercurial', which got me to the hg repo (even though it was the 5th result) first. After realizing I couldn't figure out how to check out the code from there, I went back to the first hit for that search, the opensource page, and made my way through to the wiki and from there to Getting Started and from there to Scratch Installation and from there to Downloading the code from which I grabbed bootstrap.pl, and from there to the one line I wanted: 'hg -q clone http://hg.dwscoalition.org/dw-free'. (Yes, if I knew mercurial better, I probably could have done this without following any of those steps ... but I didn't. In fact, I didn't even have mercurial installed at the time. So I also installed that.)
So, it's now 7:04, and after grep -r-ing for rpc_cuttag, I find cgi-bin/DW/Controller/RPC/CutExpander.pm, and realize that all the work in the cut expander is done by LJ::CleanHTML. My memory of working on LJ almost a decade ago was that this particular chunk of code was a hairy, nasty, mess -- as is almost any code that parses HTML -- so I was unsurprised to find that little has changed ;)
After looking briefly at CleanHTML (07:11), I decided that this was almost certainly a side effect of the particular entry text -- simple tests showed that this worked, but this entry was clearly failing. I checked briefly whether any community mods for that community might be in #dreamwidth -- the answer to which was no. (07:12).
At 07:15 I summarize my progress so far to the channel:
Around 07:40, I realize that it is possible for tags like <div class="ljcut"> to be turned into real cuts -- presumably to support the rich text editor -- and that these are handled like lj-cuts in one chunk of the code, but notably *not* in the chunk of code that counts lj-cut tags in cases where we are looking to expand cut text.
At 07:42, I reproduce the problem that the entry demonstrates in my own journal, so getting content from the original author is no longer important. There's now enough information to file a bug about this issue, so I search zilla for anything related to lj-cut and auto-expansion, and find nothing. I spend the next 15 minutes writing up the bug, bug 4435, which I finish at 07:57.
At this point, I start to poke at the code and figure out if I can actually fix the issue.
In short, the problem is that two different types of lj-cuts are treated differently by different sections of the code. The fix is to change that. The code for these is relatively simple. In short, I started going through the entire section of code looking for things that explicitly mentioned lj-cut, and trying to update those sections to take into account *both* types of lj-cut.
So (08:01), I uploaded an initial patch, but there was one lj-cut I wasn't sure if it should be handled or not, so I left a comment to that extent in the patch. I then investigated that branch -- which, specifically, was the code that puts a literal </cut> in the view that community moderators see. (This chunk of code won't display the end-cut text in cases where a user uses an RTE-inserted div-class-ljcut element.) I uploaded a new patch with a comment about why I wasn't touching that code.
I then went to my Dreamhack, which Sophie set up for me last week (Thanks Sophie!), and ran the start-apache and followed the "Create a system user" item in rah's wikipage. I then applied my patch to the CleanHTML code, and created an entry under the system account. (08:17)
Then I got confused briefly, because the code... worked on my LJ install. Then I realized that the CleanHTML perl code isn't read when Apache starts (when it was broken) but instead when it is needed (which wasn't until I opened the entry, after which I had already patched it). I reverse-applied my patch, restarted apache, and confirmed the code failed. (08:22)
On IRC, V_PauAmma_V then helped point me to the tests, under dw/t/ on my dreamhack. I couldn't actually figure out how to run them, but did realize they were Perl, and ran them that way. (Note that running tests is not actually described on any Dreamwidth wiki page I can find; I checked submitting your changes to dreamwidth, dev patches, dev version control and some others, and didn't find any place where it said how to run them.) She then pointed out that they were runnable via `prove`, so I used that.
At 08:32, I had failing tests for the problem that I was trying to fix. I applied my patch, and re-ran the tests, and they passed (08:33).
I tried to prepare a patch, but dw/t/ and dw/cgi-bin/LJ/ weren't under hg. I was going to just copy the changes back to my local machine, but instead I did `find . -name .hg` in my homedir, and found that the hg repos are under cvs/. So I copied my changes into cvs/dw-free/, and generated a diff from there. While I was doing that, I ran the tests across the entire codebase using 'prove' in the dw/ directory. (08:43).
I uploaded the patch to Bugzilla, marking it review?; V_PauAmma_V suggested I also mark it commit?. (Took me a bit to figure out how to do that, but I got it.) I then also generated a patch in hg-changeset format, in case that was preferred, and added it to the ticket. (Note that there are also no docs that I could find about how to generate a patch; there are a couple places that do say "Make a patch.", but nothing that says how.)
Total time: ~2 hours, start to finish, with a lot of stumbling around to figure out how things were supposed to work, and I'm still not particularly sure that I did things right, but it does look like I actually fixed the problem... the question is really just whether it also made anything else worse :)
First, I didn't even know how to expand LJ-cuts inline. So I started from pretty much nothing here. :) But in the end, I think I found the bug, and attached a patch to Zilla, so I figured I'd write up my process.
Start: (6:46) Okay, clicking on the cut gives the wrong thing. Open up my browser's web console, and check the HTTP request that it's sending is right -- it specifies cutid=1, but is given one from a much later cut. (6:48) To me, this suggests that the problem is server side (not Javascript), so I have to dive into code I can't just load in my browser. I experiment a bit more to ensure that this appears to be the case, and determine I can't see any evidence that this is client side. (6:58).
I Googled 'dreamwidth code' (which didn't get me very far), then 'dreamwidth mercurial', which got me to the hg repo (even though it was the 5th result) first. After realizing I couldn't figure out how to check out the code from there, I went back to the first hit for that search, the opensource page, and made my way through to the wiki and from there to Getting Started and from there to Scratch Installation and from there to Downloading the code from which I grabbed bootstrap.pl, and from there to the one line I wanted: 'hg -q clone http://hg.dwscoalition.org/dw-free'. (Yes, if I knew mercurial better, I probably could have done this without following any of those steps ... but I didn't. In fact, I didn't even have mercurial installed at the time. So I also installed that.)
So, it's now 7:04, and after grep -r-ing for rpc_cuttag, I find cgi-bin/DW/Controller/RPC/CutExpander.pm, and realize that all the work in the cut expander is done by LJ::CleanHTML. My memory of working on LJ almost a decade ago was that this particular chunk of code was a hairy, nasty, mess -- as is almost any code that parses HTML -- so I was unsurprised to find that little has changed ;)
After looking briefly at CleanHTML (07:11), I decided that this was almost certainly a side effect of the particular entry text -- simple tests showed that this worked, but this entry was clearly failing. I checked briefly whether any community mods for that community might be in #dreamwidth -- the answer to which was no. (07:12).
At 07:15 I summarize my progress so far to the channel:
- Javascript is doing fine. IT's asking for the right cut. The server side is broken.
- The server side RPC mechanism is likely doing the right thing; there's no smarts there.
- Once it gets past that, it goes into the CleanHTML code, which in addition to being the gnarliest perl code I've ever not touched, definitely behaves very differently when in "looking for cut mode"
than it does otherwise, so it's not super-surprising that this fails.
Around 07:40, I realize that it is possible for tags like <div class="ljcut"> to be turned into real cuts -- presumably to support the rich text editor -- and that these are handled like lj-cuts in one chunk of the code, but notably *not* in the chunk of code that counts lj-cut tags in cases where we are looking to expand cut text.
At 07:42, I reproduce the problem that the entry demonstrates in my own journal, so getting content from the original author is no longer important. There's now enough information to file a bug about this issue, so I search zilla for anything related to lj-cut and auto-expansion, and find nothing. I spend the next 15 minutes writing up the bug, bug 4435, which I finish at 07:57.
At this point, I start to poke at the code and figure out if I can actually fix the issue.
In short, the problem is that two different types of lj-cuts are treated differently by different sections of the code. The fix is to change that. The code for these is relatively simple. In short, I started going through the entire section of code looking for things that explicitly mentioned lj-cut, and trying to update those sections to take into account *both* types of lj-cut.
So (08:01), I uploaded an initial patch, but there was one lj-cut I wasn't sure if it should be handled or not, so I left a comment to that extent in the patch. I then investigated that branch -- which, specifically, was the code that puts a literal </cut> in the view that community moderators see. (This chunk of code won't display the end-cut text in cases where a user uses an RTE-inserted div-class-ljcut element.) I uploaded a new patch with a comment about why I wasn't touching that code.
I then went to my Dreamhack, which Sophie set up for me last week (Thanks Sophie!), and ran the start-apache and followed the "Create a system user" item in rah's wikipage. I then applied my patch to the CleanHTML code, and created an entry under the system account. (08:17)
Then I got confused briefly, because the code... worked on my LJ install. Then I realized that the CleanHTML perl code isn't read when Apache starts (when it was broken) but instead when it is needed (which wasn't until I opened the entry, after which I had already patched it). I reverse-applied my patch, restarted apache, and confirmed the code failed. (08:22)
On IRC, V_PauAmma_V then helped point me to the tests, under dw/t/ on my dreamhack. I couldn't actually figure out how to run them, but did realize they were Perl, and ran them that way. (Note that running tests is not actually described on any Dreamwidth wiki page I can find; I checked submitting your changes to dreamwidth, dev patches, dev version control and some others, and didn't find any place where it said how to run them.) She then pointed out that they were runnable via `prove`, so I used that.
At 08:32, I had failing tests for the problem that I was trying to fix. I applied my patch, and re-ran the tests, and they passed (08:33).
I tried to prepare a patch, but dw/t/ and dw/cgi-bin/LJ/ weren't under hg. I was going to just copy the changes back to my local machine, but instead I did `find . -name .hg` in my homedir, and found that the hg repos are under cvs/. So I copied my changes into cvs/dw-free/, and generated a diff from there. While I was doing that, I ran the tests across the entire codebase using 'prove' in the dw/ directory. (08:43).
I uploaded the patch to Bugzilla, marking it review?; V_PauAmma_V suggested I also mark it commit?. (Took me a bit to figure out how to do that, but I got it.) I then also generated a patch in hg-changeset format, in case that was preferred, and added it to the ticket. (Note that there are also no docs that I could find about how to generate a patch; there are a couple places that do say "Make a patch.", but nothing that says how.)
Total time: ~2 hours, start to finish, with a lot of stumbling around to figure out how things were supposed to work, and I'm still not particularly sure that I did things right, but it does look like I actually fixed the problem... the question is really just whether it also made anything else worse :)