-
Notifications
You must be signed in to change notification settings - Fork 30.2k
[FIX] html_editor: properly handle ctrl+click and link preview in editor
#188455
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
[FIX] html_editor: properly handle ctrl+click and link preview in editor
#188455
Conversation
dd25f68 to
768b514
Compare
ee21b04 to
adfd795
Compare
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.
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.
Hello @walidsahli Thanks for the PR! I think @Mtaylorr has a very good point, we should only process the preview when it's not an abstract model. To do this I suggest to add other conditions in the if check:
| if action_name in request.env: | |
| if (action_name.startswith('m-') or '.' in action_name) and action_name in request.env and not request.env[action_name]._abstract: |
By doing this, we ensure we don't access abstract models and also we restrict the action_name in this case to be a model name
cc @dmo-odoo
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.
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.
Here since it's actually a model's name we are using, it better to use model_name instead. Also, we cover another possibility of model name in the URL which starts with m-
| model = request.env[action_name].with_context(context) | |
| model_name = action_name.removeprefix('m-') | |
| model = request.env[model_name].with_context(context) |
243015b to
7aacef6
Compare
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.
@walidsahli , can you add a test for abstract model here to make sure he can't access it
7aacef6 to
dbfa998
Compare
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.
Thanks for your work! I wrote a few comments.
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.
Since both the issues and their respective fixes are completely unrelated from each other, they should be fixed in separated commits. It is ok to keep them in a single PR.
In the commit message for the _blank fix, it is better to link to the actual commit introducing the fix rather than on an arbitrary line of code in 17.0.
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.
As you can see from this red sign on GitHub, the file is missing the empty line at the end.
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.
Is this supposed to be a comment? Because right now, it's a string that just floats there in the middle of the code... ^^' I looked in Odoo's codebase (I haven't worked in Python for years) and it seems this kind of triple quoted strings are only used as comments for the docstring of functions (as per PEP 257), that is right after the function definition, but never in the middle of the code itself. Classical Python comments use #.
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.
Of course, i'm sorry yea it should be a comment not a string.
dd0404f to
2d1e9a6
Compare
**Problem**: Internal links with the format `odoo/<model>/<record_id>` do not display a proper preview. Instead, a warning toaster appears: _"Action `<model>` not found, link preview is not available. Please check your URL is correct."_ **Solution**: For internal links (`odoo/<model>/<record_id>`), validate the `action_name` as a model name. If valid, treat `action_name` as the model name for the link preview. **Steps to reproduce**: 1. Open the editor. 2. Add a link with the path `odoo/project.task/59`. 3. Press `Ctrl+click`. 4. Observe the warning toaster: _"Action project.task not found, link preview is not available. Please check your URL is correct."_ opw-4353235
**Problem**: `Ctrl+click` on a link in the editor does not open the link in a new tab. **Solution**: Implement the same code as version 17.0 to handle `ctrl+click` for opening links in a new tab: odoo@981290e **Steps to reproduce**: 1. Open the editor. 2. Add a link with the path `odoo/project.task/59`. 3. Try `Ctrl+click`: - **Issue**: The link does not open in a new tab. opw-4353235
2d1e9a6 to
d854987
Compare
|
@robodoo rebase-ff r+ |
|
Merge method set to rebase and fast-forward. |
**Problem**: Internal links with the format `odoo/<model>/<record_id>` do not display a proper preview. Instead, a warning toaster appears: _"Action `<model>` not found, link preview is not available. Please check your URL is correct."_ **Solution**: For internal links (`odoo/<model>/<record_id>`), validate the `action_name` as a model name. If valid, treat `action_name` as the model name for the link preview. **Steps to reproduce**: 1. Open the editor. 2. Add a link with the path `odoo/project.task/59`. 3. Press `Ctrl+click`. 4. Observe the warning toaster: _"Action project.task not found, link preview is not available. Please check your URL is correct."_ opw-4353235 Part-of: #188455 Signed-off-by: David Monjoie (dmo) <dmo@odoo.com>
**Problem**: Internal links with the format `odoo/<model>/<record_id>` do not display a proper preview. Instead, a warning toaster appears: _"Action `<model>` not found, link preview is not available. Please check your URL is correct."_ **Solution**: For internal links (`odoo/<model>/<record_id>`), validate the `action_name` as a model name. If valid, treat `action_name` as the model name for the link preview. **Steps to reproduce**: 1. Open the editor. 2. Add a link with the path `odoo/project.task/59`. 3. Press `Ctrl+click`. 4. Observe the warning toaster: _"Action project.task not found, link preview is not available. Please check your URL is correct."_ opw-4353235 Part-of: odoo/odoo#188455 Signed-off-by: David Monjoie (dmo) <dmo@odoo.com>
**Problem**: `Ctrl+click` on a link in the editor does not open the link in a new tab. **Solution**: Implement the same code as version 17.0 to handle `ctrl+click` for opening links in a new tab: odoo/odoo@981290e **Steps to reproduce**: 1. Open the editor. 2. Add a link with the path `odoo/project.task/59`. 3. Try `Ctrl+click`: - **Issue**: The link does not open in a new tab. opw-4353235 closes odoo/odoo#188455 Signed-off-by: David Monjoie (dmo) <dmo@odoo.com>

Problem:
Ctrl+clickon a link in the editor does not open the link in a new tab.odoo/<model>/<record_id>do not display a proper preview. Instead, a warning toaster appears: "Action<model>not found, link preview is not available. Please check your URL is correct."Solution:
ctrl+clickfor opening links in a new tab: 981290eodoo/<model>/<record_id>), validate theaction_nameas a model name. If valid, treataction_nameas the model name for the link preview.Steps to reproduce:
odoo/project.task/59.Ctrl+click:opw-4353235
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr