|
|
Created:
7 years, 8 months ago by justinlin Modified:
7 years, 8 months ago CC:
chromium-reviews, Aaron Boodman, chromium-apps-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Visibility:
Public. |
DescriptionUpdate tabCapture API docs with browser action info.
BUG=230911
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=195178
Patch Set 1 #
Total comments: 6
Patch Set 2 : #
Total comments: 5
Patch Set 3 : #Patch Set 4 : #
Messages
Total messages: 16 (0 generated)
missed updating the docs
https://codereview.chromium.org/14107010/diff/1/chrome/common/extensions/api/... File chrome/common/extensions/api/tab_capture.idl (right): https://codereview.chromium.org/14107010/diff/1/chrome/common/extensions/api/... chrome/common/extensions/api/tab_capture.idl:51: // of this method for the currently active tab must also first be "granted" must also first be -> can only be "granted" -> granted user action of clicking on the extension icon -> user action and then link to the activeTab docs :)
https://codereview.chromium.org/14107010/diff/1/chrome/common/extensions/api/... File chrome/common/extensions/api/tab_capture.idl (right): https://codereview.chromium.org/14107010/diff/1/chrome/common/extensions/api/... chrome/common/extensions/api/tab_capture.idl:51: // of this method for the currently active tab must also first be "granted" Might say: Use of this method requires that the user first grants permission for the active tab by clicking on the extension's browser action. Question: Is the same permission granted by clicking on a page action?
https://codereview.chromium.org/14107010/diff/1/chrome/common/extensions/api/... File chrome/common/extensions/api/tab_capture.idl (right): https://codereview.chromium.org/14107010/diff/1/chrome/common/extensions/api/... chrome/common/extensions/api/tab_capture.idl:51: // of this method for the currently active tab must also first be "granted" > Question: > Is the same permission granted by clicking on a page action? > Yes - and several other user actions.
https://codereview.chromium.org/14107010/diff/1/chrome/common/extensions/api/... File chrome/common/extensions/api/tab_capture.idl (right): https://codereview.chromium.org/14107010/diff/1/chrome/common/extensions/api/... chrome/common/extensions/api/tab_capture.idl:51: // of this method for the currently active tab must also first be "granted" On 2013/04/18 18:31:36, kalman wrote: > > > Question: > > Is the same permission granted by clicking on a page action? > > > > Yes - and several other user actions. Okay, then might say "clicking on the extension" or some other generic phrase in the suggestion above.
http://developer.chrome.com/extensions/activeTab.html may be a good reference for language.
Updated the error message as well. https://codereview.chromium.org/14107010/diff/1/chrome/common/extensions/api/... File chrome/common/extensions/api/tab_capture.idl (right): https://codereview.chromium.org/14107010/diff/1/chrome/common/extensions/api/... chrome/common/extensions/api/tab_capture.idl:51: // of this method for the currently active tab must also first be "granted" On 2013/04/18 18:29:21, kalman wrote: > must also first be -> can only be > > "granted" -> granted > > user action of clicking on the extension icon -> user action > > and then link to the activeTab docs :) Done. Yea, that sounds better. https://codereview.chromium.org/14107010/diff/1/chrome/common/extensions/api/... chrome/common/extensions/api/tab_capture.idl:51: // of this method for the currently active tab must also first be "granted" On 2013/04/18 18:33:23, mark a. foltz wrote: > On 2013/04/18 18:31:36, kalman wrote: > > > > > Question: > > > Is the same permission granted by clicking on a page action? > > > > > > > Yes - and several other user actions. > > Okay, then might say "clicking on the extension" or some other generic phrase in > the suggestion above. Just "granted through a user action" like Ben suggested?
If you agree (or mostly agree) with me, lgtm. https://codereview.chromium.org/14107010/diff/9001/chrome/common/extensions/a... File chrome/common/extensions/api/tab_capture.idl (right): https://codereview.chromium.org/14107010/diff/9001/chrome/common/extensions/a... chrome/common/extensions/api/tab_capture.idl:50: // Extensions must have the "tabCapture" permission to use this method. Use nit: "extensions must have the tabCapture permission to use this method" is implied by this being the tabCapture API. I'd just delete that sentence. https://codereview.chromium.org/14107010/diff/9001/chrome/common/extensions/a... chrome/common/extensions/api/tab_capture.idl:52: // through a user action. See: "User action" isn't quite accurate; it might imply that the API calls can only be made during a user action, when in fact they can be made at any time during of after a user action. Something like "This method can only be used on a page which the extension has been invoked on, similar to the way that <a href="activeTab">activeTab</a> works." https://codereview.chromium.org/14107010/diff/9001/chrome/common/extensions/a... chrome/common/extensions/api/tab_capture.idl:53: // http://developer.chrome.com/extensions/activeTab.html could you make this <a href="activeTab.html">activeTab</a>
LGTM assuming final wording is worked out with kalman.
thanks https://codereview.chromium.org/14107010/diff/9001/chrome/common/extensions/a... File chrome/common/extensions/api/tab_capture.idl (right): https://codereview.chromium.org/14107010/diff/9001/chrome/common/extensions/a... chrome/common/extensions/api/tab_capture.idl:50: // Extensions must have the "tabCapture" permission to use this method. Use On 2013/04/18 20:17:13, kalman wrote: > nit: "extensions must have the tabCapture permission to use this method" is > implied by this being the tabCapture API. I'd just delete that sentence. Done. https://codereview.chromium.org/14107010/diff/9001/chrome/common/extensions/a... chrome/common/extensions/api/tab_capture.idl:52: // through a user action. See: On 2013/04/18 20:17:13, kalman wrote: > "User action" isn't quite accurate; it might imply that the API calls can only > be made during a user action, when in fact they can be made at any time during > of after a user action. > > Something like "This method can only be used on a page which the extension has > been invoked on, similar to the way that <a href="activeTab">activeTab</a> > works." Done, something similar.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/justinlin@chromium.org/14107010/18001
Presubmit check for 14107010-18001 failed and returned exit status 1. INFO:root:Found 3 file(s). Traceback (most recent call last): File "docs/server2/integration_test.py", line 6, in <module> from handler import Handler File "/b/commit-queue/workdir/chromium/chrome/common/extensions/docs/server2/handler.py", line 13, in <module> from server_instance import ServerInstance File "/b/commit-queue/workdir/chromium/chrome/common/extensions/docs/server2/server_instance.py", line 9, in <module> from api_data_source import APIDataSource File "/b/commit-queue/workdir/chromium/chrome/common/extensions/docs/server2/api_data_source.py", line 10, in <module> import third_party.json_schema_compiler.json_parse as json_parse ImportError: No module named third_party.json_schema_compiler.json_parse Traceback (most recent call last): File "docs/server2/link_converter.py", line 15, in <module> import third_party.json_schema_compiler.model as model ImportError: No module named third_party.json_schema_compiler.model Running presubmit commit checks ... Running /b/commit-queue/workdir/chromium/PRESUBMIT.py Running /b/commit-queue/workdir/chromium/chrome/PRESUBMIT.py Running /b/commit-queue/workdir/chromium/chrome/browser/extensions/PRESUBMIT.py Running /b/commit-queue/workdir/chromium/chrome/common/extensions/PRESUBMIT.py Traceback (most recent call last): File "/b/commit-queue/verification/presubmit_shim.py", line 33, in <module> sys.exit(presubmit_support.Main(argv)) File "/b/depot_tools/presubmit_support.py", line 1305, in Main rietveld_obj) File "/b/depot_tools/presubmit_support.py", line 1129, in DoPresubmitChecks results += executer.ExecPresubmitScript(presubmit_script, filename) File "/b/depot_tools/presubmit_support.py", line 1046, in ExecPresubmitScript result = eval(function_name + '(*__args)', context) File "<string>", line 1, in <module> File "<string>", line 173, in CheckChangeOnCommit File "<string>", line 166, in _CheckChange File "<string>", line 134, in _CheckLinks File "/b/depot_tools/subprocess2.py", line 449, in check_output return check_call_out(args, stdout=PIPE, **kwargs)[0] File "/b/depot_tools/subprocess2.py", line 412, in check_call_out returncode, args, kwargs.get('cwd'), out[0], out[1]) subprocess2.CalledProcessError: Command docs/server2/link_converter.py -o -f /b/commit-queue/workdir/chromium/chrome/common/extensions/api/tab_capture.idl returned non-zero exit status 1 in /b/commit-queue/workdir/chromium/chrome/common/extensions
On 2013/04/18 21:07:32, I haz the power (commit-bot) wrote: > Presubmit check for 14107010-18001 failed and returned exit status 1. > > INFO:root:Found 3 file(s). > > Traceback (most recent call last): > File "docs/server2/integration_test.py", line 6, in <module> > from handler import Handler > File > "/b/commit-queue/workdir/chromium/chrome/common/extensions/docs/server2/handler.py", > line 13, in <module> > from server_instance import ServerInstance > File > "/b/commit-queue/workdir/chromium/chrome/common/extensions/docs/server2/server_instance.py", > line 9, in <module> > from api_data_source import APIDataSource > File > "/b/commit-queue/workdir/chromium/chrome/common/extensions/docs/server2/api_data_source.py", > line 10, in <module> > import third_party.json_schema_compiler.json_parse as json_parse > ImportError: No module named third_party.json_schema_compiler.json_parse > Traceback (most recent call last): > File "docs/server2/link_converter.py", line 15, in <module> > import third_party.json_schema_compiler.model as model > ImportError: No module named third_party.json_schema_compiler.model > Running presubmit commit checks ... > Running /b/commit-queue/workdir/chromium/PRESUBMIT.py > Running /b/commit-queue/workdir/chromium/chrome/PRESUBMIT.py > Running /b/commit-queue/workdir/chromium/chrome/browser/extensions/PRESUBMIT.py > Running /b/commit-queue/workdir/chromium/chrome/common/extensions/PRESUBMIT.py > Traceback (most recent call last): > File "/b/commit-queue/verification/presubmit_shim.py", line 33, in <module> > sys.exit(presubmit_support.Main(argv)) > File "/b/depot_tools/presubmit_support.py", line 1305, in Main > rietveld_obj) > File "/b/depot_tools/presubmit_support.py", line 1129, in DoPresubmitChecks > results += executer.ExecPresubmitScript(presubmit_script, filename) > File "/b/depot_tools/presubmit_support.py", line 1046, in ExecPresubmitScript > result = eval(function_name + '(*__args)', context) > File "<string>", line 1, in <module> > File "<string>", line 173, in CheckChangeOnCommit > File "<string>", line 166, in _CheckChange > File "<string>", line 134, in _CheckLinks > File "/b/depot_tools/subprocess2.py", line 449, in check_output > return check_call_out(args, stdout=PIPE, **kwargs)[0] > File "/b/depot_tools/subprocess2.py", line 412, in check_call_out > returncode, args, kwargs.get('cwd'), out[0], out[1]) > subprocess2.CalledProcessError: Command docs/server2/link_converter.py -o -f > /b/commit-queue/workdir/chromium/chrome/common/extensions/api/tab_capture.idl > returned non-zero exit status 1 in > /b/commit-queue/workdir/chromium/chrome/common/extensions Ugh, sorry, I broke that. Fixing...
fixed
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/justinlin@chromium.org/14107010/18001
Message was sent while issue was closed.
Change committed as 195178 |