Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
14107010:
Update tabCapture API docs with browser action info. (Closed)
|
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 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
