Bug 51142

Summary: size status bubble to it's contents on the code review page
Product: WebKit Reporter: Ojan Vafai <ojan>
Component: New BugsAssignee: Ojan Vafai <ojan>
Status: RESOLVED FIXED    
Severity: Normal CC: ossy, tonikitoo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch abarth: review+

Description Ojan Vafai 2010-12-15 15:17:00 PST
size status bubble to it's contents on the code review page
Comment 1 Ojan Vafai 2010-12-15 15:17:32 PST
Created attachment 76692 [details]
Patch
Comment 2 Ojan Vafai 2010-12-15 15:18:25 PST
Created attachment 76693 [details]
Patch
Comment 3 Adam Barth 2010-12-15 15:23:38 PST
Comment on attachment 76693 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=76693&action=review

> BugsSite/code-review.js:259
> +    $('.statusBubble')[0].style.height = e.data.height;
> +    $('.statusBubble')[0].style.width = e.data.width;

This won't work in older browsers, but ok.

> BugsSite/code-review.js:289
> +      // Can't append the HTML because we need to set the onload handler before appending the iframe to the DOM.
> +      statusBubble.onload = handleStatusBubbleLoad;

You could use a live event handler, but this seems fine.
Comment 4 Ojan Vafai 2010-12-15 16:03:00 PST
Committed r74160: <http://trac.webkit.org/changeset/74160>
Comment 5 Csaba Osztrogonác 2010-12-17 06:24:06 PST
Reopen, because it is buggy:

After this patch, code review page doesn't work for me with Firefox,
I see the pretty diff, but I can't comment the source and I can't 
see the cq, review flags.

I tried with Chrome too. It works, but the 3 lines 
height textarea of the overall comment is so ugly.
Comment 6 Ojan Vafai 2010-12-17 08:04:37 PST
(In reply to comment #5)
> After this patch, code review page doesn't work for me with Firefox,
> I see the pretty diff, but I can't comment the source and I can't 
> see the cq, review flags.

I'll try to fix this this morning, but I leave on vacation in a couple hours. I didn't realize anyone on the WebKit project did code reviews in FF. If I don't get it in by then, it will be about 2 weeks before I can get back to it.

> I tried with Chrome too. It works, but the 3 lines 
> height textarea of the overall comment is so ugly.

It's hard to work with "ugly" as feedback. Patches are welcome.
Comment 7 Antonio Gomes 2010-12-17 08:32:58 PST
(In reply to comment #6)
> (In reply to comment #5)
> > After this patch, code review page doesn't work for me with Firefox,
> > I see the pretty diff, but I can't comment the source and I can't 
> > see the cq, review flags.
> 
> I'll try to fix this this morning, but I leave on vacation in a couple hours. I didn't realize anyone on the WebKit project did code reviews in FF. If I don't get it in by then, it will be about 2 weeks before I can get back to it.

I do reviewed on either Firefox or Chromium or Safari, whatever is easily accessible :)

> > I tried with Chrome too. It works, but the 3 lines 
> > height textarea of the overall comment is so ugly.

I also agree here, Ojan :( ... Not sure what I could feedback to you other than a visual adjective.
Comment 8 Ojan Vafai 2010-12-17 10:41:48 PST
OK. It's now functional in Firefox after http://trac.webkit.org/changeset/74276 and http://trac.webkit.org/changeset/74277. It's not pretty though. I assume it must be a difference between webkit's and Firefox's flexbox implementations.

Not planning on working on it more unless there's significant demand for this tool to work well in Firefox. Happy to do reviews for it though. :)

It's hard to develop on the tool for Firefox because the way we do so is via a Chrome extension. Hope this is good enough.