-
Notifications
You must be signed in to change notification settings - Fork 195
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chromium: Add chrome-virtual-keyboard recipe #503
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rework the following items I mentioned below... also, consider using the variables with _
instead of -
as this is the commonly used pattern.
meta-chromium/recipes-browser/chromium-extensions/chrome-virtual-keyboard_git.bb
Outdated
Show resolved
Hide resolved
40d4a9b
to
7524659
Compare
@otavio: @kraj did advise against merging it in the issue #490 if no one is interested in testing it. obviously windriver is interested in maintaining/fixing any issues, but I don't expect this extension to be a long-term solution for us if the internal requirements don't change (ex: extensive multi-language support in the virtual keyboard) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kraj why you think this shouldn't be merged?
meta-chromium/recipes-browser/chromium-extensions/chrome-virtual-keyboard_git.bb
Outdated
Show resolved
Hide resolved
Tested and works fine for me.
|
@otavio is there still a change that you are looking for? Github thinks there is. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I hadn't looked at this PR before and have a few concerns with its current shape.
The way the chromium recipe itself needs to be modified looks too intrusive (I don't think there should be a PACKAGECONFIG
knob for each extension, and the way CHROMIUM_EXTENSIONS
is defined requires knowing about each extension again), and I'd rather go with an approach that:
- Documents the extensions path in README.md (or optionally defines it in a class file like the PR currently does)
- Changes the
chromium
shell script wrapper to look for all directories under this extension path and derive the list of extensions to load at runtime.
In any case, one thing that's not entirely clear to me yet is the advantage of this approach of packaging and shipping an extension versus using e.g. a policy to cause it to be installed (and updated automatically) or a master_policy file.
@rakuco, I agree about the suggestion to walk in the extension dir and enable the extensions. About packaging it versus enabling them with a policy, I'd argue that being it an embedded device, I'd rather prefer to know what is running and don't have it is automatically updated as it might impose behavior changes or introduce regressions. |
I'm not very familiar with deploying extensions with policies or preferences myself, but I think it's possible not to have them be updated automatically. It'd still be installed at runtime (whatever version is available at the time), not sure if that's also too bad from an embedded device perspective. |
What if the device does not have network access? |
...or has a read-only rootfs... |
I thought it wouldn't be the case for people shipping a web browser and requiring one or more extensions, but I really have no idea if that's true or not.
But |
@rakuco, in fact, it is a widespread use case to provide the chromium with no network access (an industrial UI, internal network kiosk, etc...). This doesn't mean we cannot make use of extensions. As @diegorondini pointed out, the read-only rootfs is another use case. In this scenario, we'd use an overlay with tmpfs for the Chromium cache, etc., but nothing would be persistent. |
Understood, thanks for clarifying. I guess we just need to generalize the approach in this PR to land it then. |
The notion of introducing an explicit policy to install extensions from a directory based on command-line arguments seems like it goes a bit far. I think we're all agreed that embedded devices are a controlled usecase. What this PR does is install extensions to the filesystem, the "install" them to a Chromium profile by passing command-line arguments, based on them being present in a directory. In our case, we're doing something similar, but in a much simplified fashion: just drop the crx and JSON file for the extension into /usr/share/chromium/extensions, and they will automatically be available when you start the browser. Neither the Chromium startup script nor recipe need any changes to make this work. |
Ah, that sounds much nicer, thanks for pointing it out. I started this review assuming something like this didn't work with Chromium, but I should've known better :-) |
So it seems, this PR should be reworked to provide a package but reusing the existing infra to enable the extension. @yifan19 can you look at this? |
yop |
ok it makes the process much simpler if this method works. update: i wasn't able to get it to automatically load. |
@@ -327,6 +327,7 @@ GN_ARGS:append:libc-musl = ' use_allocator_shim=false use_allocator="none"' | |||
CHROMIUM_EXTRA_ARGS ?= " \ | |||
${@bb.utils.contains('PACKAGECONFIG', 'use-egl', '--use-gl=egl', '', d)} \ | |||
${@bb.utils.contains('PACKAGECONFIG', 'kiosk-mode', '--kiosk --no-first-run --incognito', '', d)} \ | |||
--load-extension=\$(find ${CHROMIUM_EXTENSION_DIR} -maxdepth 1 | sed 1d | tr '\\\\n' , ) \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In /usr/share/chromium/extensions on my device I have a set of .json and .crx files (packaged and signed extensions). This snippet will pick up those extensions and add all of the JSON and crx files to the command-line.
This doesn't seem to have any ill effect, but it's unnecessary. I've also found that this means that there's a trailing ,
in the load-extension command-line argument.
A simple solution to adding the files is to add -type d
to the sed command.
A simple solution to the trailing ,
is to add the following to the pipeline: sed '$ s/,$//'
What ended up working for me was:
--load-extension=$(find /usr/share/chromium/extensions -type d -maxdepth 1 | sed 1d | tr '\n' , | sed '$ s/,$//' )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
trailing ,
won't affect loading the extension when I tested it.
A simple solution to adding the files is to add -type d to the sed command.
ya -type d
is a good idea
In /usr/share/chromium/extensions on my device I have a set of .json and .crx files (packaged and signed extensions). This snippet will pick up those extensions and add all of the JSON and crx files to the command-line.
this is scenario where you have already a packaged extension, which I don't think bitbake can do (it seems to be a zip file)
79bce74
to
48e97d5
Compare
chrome-virtual-keyboard is an extension to chromium that has a javascript on-screen keyboard that allow users to use chromium without a physical keyboard. According to chrome webstore, there are over 700K users. limitations: * upstream hasn't maintained this extension for a long time * not all textbox will work, notably iframes leveraging the chromium flag: --extension-load="" to load a local unpacked extension. Signed-off-by: Yi Fan Yu <[email protected]> Signed-off-by: Yi Fan Yu <[email protected]>
This feature appears to have been abandoned, is there still any interest in merging it in? We have a use case for which it would be applicable: an embedded device where chromium is used to provide a user interface to our firmware. Many of these devices will be in installed in locations where they will never be connected to the internet or where network policies would strictly prohibit them from installing extensions at runtime. Furthermore for both QA and regulatory compliance purposes we like our systems to be completely defined at build time so that the image we ship is always exactly what is on the device. We would be very leary of including anything that would require any component (even something as simple as a keyboard extension) to be installed on a running system. I have tested the code changes from this PR on a local fork of commit 633dbee (latest verified version) and they appear to work just fine on our device. If there is interest in merging this feature in I would be willing to take a look at any outstanding requests on this PR and see if I can resolve them. |
I personally would like to have this feature but my maintainer hat says it will be quite a bit of drag if upstream is not supporting it. Since we will have to keep forward porting it and I am not sure how much of the work we will be imposing on community. |
Wouldn't we be only supporting the extension loading mechanism? I don't think that's going to change much in the future. Ensuring the extensions that are loaded keep working with new versions is the responsibility of the extensions' maintainers, isn't it? |
Yes, that would seem to be reasonable: keep the extension loading mechanism which is supported upstream but not code for any extension in particular. Something could be added to the README describing how to use the mechanism but it is up to whomever wants to use a particular extension to actually incorporate and maintain it. |
I was an intern working on this kiosk chromium feature, Also very happy to see people interested in this feature |
Sorry, was out of office for a while and have not been keeping up with this. Any desire to move this forward as a framework for generic extension support only and, if so, who is able to do the work (both yifan19 and I are willing I think)? I discovered as part of my testing that starting chromium in incognito mode automatically disables extensions but, as currently implemented here, starting chromium in kiosk mode automatically invokes incognito mode. To make this feature useful in kiosk mode we will need to remove incognito mode from the kiosk mode arguments and break it out as a separate argument. |
enabled through PACKAGECONFIG vkeyboard-extension
Signed-off-by: Yi Fan Yu [email protected]