Bug 53554

Summary: make draft comments focusable
Product: WebKit Reporter: Ojan Vafai <ojan>
Component: New BugsAssignee: Ojan Vafai <ojan>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch abarth: review+

Ojan Vafai
Reported 2011-02-01 16:54:26 PST
make draft comments focusable
Attachments
Patch (3.26 KB, patch)
2011-02-01 16:55 PST, Ojan Vafai 		abarth: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Ojan Vafai
Comment 1 2011-02-01 16:55:57 PST
Created attachment 80856 [details] Patch
Adam Barth
Comment 2 2011-02-01 21:30:05 PST
Comment on attachment 80856 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=80856&action=review This is fine, but consider the comments below. BTW, I love j/k. :) > Websites/bugs.webkit.org/code-review.js:1280 > - if (node.hasClass(className)) { > + if (focus_type == FOCUS_TYPE.COMMENT && (node.hasClass('frozenComment') || node.hasClass('previousComment')) || > + focus_type == FOCUS_TYPE.DIFF_BLOCK && node.hasClass('DiffBlock')) { I'd just pass in a function that you call at this branch. That stops this code from having to know about all the different types.
Ojan Vafai
Comment 3 2011-02-01 22:00:34 PST
Committed r77363: <http://trac.webkit.org/changeset/77363>
Ojan Vafai
Comment 4 2011-02-01 22:03:17 PST
Comment on attachment 80856 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=80856&action=review >> Websites/bugs.webkit.org/code-review.js:1280 >> + focus_type == FOCUS_TYPE.DIFF_BLOCK && node.hasClass('DiffBlock')) { > > I'd just pass in a function that you call at this branch. That stops this code from having to know about all the different types. Done. That's much better.
Note You need to log in before you can comment on or make changes to this bug.