Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 5938722247802880: Support the new findbar API (Closed)

Created:
March 5, 2014, 9:51 p.m. by Felix Dahlke
Modified:
March 18, 2014, 4:07 p.m.
Reviewers:
Wladimir Palant
Visibility:
Public.

Description

Firefox 27 introduced a new (cleaner) findbar API, replacing the old one that required faking a browser. With this patch, we support both. Tested in Firefox 17 and 27.

Patch Set 1 : #

Total comments: 16

Patch Set 2 : Fixed issues #

Patch Set 3 : Added remaining stubs #

Unified diffs Side-by-side diffs Delta from patch set Stats (+82 lines, -5 lines) Patch
M chrome/content/ui/filters-search.js View 1 2 1 chunk +82 lines, -5 lines 0 comments Download

Messages

Total messages: 6
Felix Dahlke
March 5, 2014, 9:52 p.m. (2014-03-05 21:52:53 UTC) #1
Wladimir Palant
http://codereview.adblockplus.org/5938722247802880/diff/5649050225344512/chrome/content/ui/filters-search.js File chrome/content/ui/filters-search.js (right): http://codereview.adblockplus.org/5938722247802880/diff/5649050225344512/chrome/content/ui/filters-search.js#newcode41 chrome/content/ui/filters-search.js:41: } Does this check make sense? From the look ...
March 6, 2014, 1:20 p.m. (2014-03-06 13:20:51 UTC) #2
Felix Dahlke
Addressed all issues. http://codereview.adblockplus.org/5938722247802880/diff/5649050225344512/chrome/content/ui/filters-search.js File chrome/content/ui/filters-search.js (right): http://codereview.adblockplus.org/5938722247802880/diff/5649050225344512/chrome/content/ui/filters-search.js#newcode41 chrome/content/ui/filters-search.js:41: } On 2014/03/06 13:20:51, Wladimir Palant ...
March 10, 2014, 10:19 p.m. (2014-03-10 22:19:55 UTC) #3
Wladimir Palant
http://codereview.adblockplus.org/5938722247802880/diff/5649050225344512/chrome/content/ui/filters-search.js File chrome/content/ui/filters-search.js (right): http://codereview.adblockplus.org/5938722247802880/diff/5649050225344512/chrome/content/ui/filters-search.js#newcode179 chrome/content/ui/filters-search.js:179: focusContent: function() {} Yes, we definitely need stubs at ...
March 13, 2014, 11:40 a.m. (2014-03-13 11:40:01 UTC) #4
Felix Dahlke
http://codereview.adblockplus.org/5938722247802880/diff/5649050225344512/chrome/content/ui/filters-search.js File chrome/content/ui/filters-search.js (right): http://codereview.adblockplus.org/5938722247802880/diff/5649050225344512/chrome/content/ui/filters-search.js#newcode179 chrome/content/ui/filters-search.js:179: focusContent: function() {} On 2014/03/13 11:40:01, Wladimir Palant wrote: ...
March 18, 2014, 3:51 p.m. (2014-03-18 15:51:50 UTC) #5
Wladimir Palant
March 18, 2014, 4:04 p.m. (2014-03-18 16:04:26 UTC) #6
LGTM

Powered by Google App Engine
This is Rietveld