Thanks for taking a look and trying it out. All good comments.

The Close menu issue was just an oversight. I tested clicking elsewhere on the page but I didn't test the Close menu. You're right that a hover might just be better.

You're right about the Download Patch being inconsistent. I'll change it to Download Diff. I can also put it on all relevant pages.

There's probably a few browser-specific quirks left to fix. I'm going to go through and test IE6/7 and Opera before this goes in to make sure they work correctly. The issue with IE7 is probably a new regression due to another fix I put in recently.

I don't know why expanding hidden lines would suddenly break but I'll look at it.

As for the replying in the diff viewer, it's definitely not perfect. I talked to David about this earlier in the development and he said the same thing. I have a plan for improving this (and I'd love your opinion on it) but it won't be part of this change. It's already massive as it is. I'm going to work on a better replying mechanism and put that up for review after this one goes in.

So my plan for replying is to have the comment sidebar work sort of like tabs. If there's existing comments, that pane will display and there will be a hard-coded entry at the top saying "New Comment" or something. It will be green, same as the comment area. The existing comments will be clickable and, when clicked, they'll appear as selected and the comment area will turn blue, along with text saying it's a reply. That way, commenting can be done right there, and it's hopefully more clear as to what this comment is: A brand new comment, or a reply to something.

I don't know if that was a good enough description of this... I'll try to reword it or do a mockup if not.

What do you think?

For the expanding lines, FireBug shows an error message in some of your new code, so I imagine it should be easy to track down.  Something about not having a replace method on a retrieved object...

I think I understand your new idea, but it would still be good to see some mockups.  I'm picturing it as the comment dialog box having some pullout tabs on the left, a green one for a new comment, and blue ones for each existing comment with author, date, and a brief part of the actual comment.  When selected, their comment can be shown in full in the main part of the dialog, with the edit box beneath it so it feels like you're replying.

I'll work on something for this.

It's all spaces, and it renders fine in Review Board for me. I remember you had this issue once before. Can't remember if we figured it out, though.

Fixed.

How so?

Will do.

Yep.

Yep, I plan to.

Fixed this and the other translatables.

Fixed.

It's possible that some people are using ?query in third party code. Unlikely, but possible. At 1.0, I think we should get rid of support for it and require ?q.

Shocked at how badly I screwed some of those up.

Yep, removed.

I initially was going to do some work for integrating firebug support with error banners, but decided against it. Removing this.

Strange.. Will fix.

Hrm, I haven't seen this. Do you know of a good repro case? Is it all files? Also, what browser?

I'll make the change to specifically set dataType: "html".

False alarm -- turns out I had a caching problem.  After I restarted memcached, all is good.

I guess the cache contents of the diff needed to change, even though the cache lookup key remains the same.  Is there a way that you can put some kind of versioning in the key to prevent this sort of accident?  Or maybe I should just always remember to clear the cache...

We could version it. We need to come up with a good plan for that and we also need to be careful about hitting the max key length.

This change is big enough that I think people will just have to clear the cache anyway, but we should definitely look into key versioning. Could even just have a global value in settings that is prepended to each key.

Good points. Sadly, I can't use defaultdict because it's not available in Python 2.4, but the first works well enough.