[MERGE] *: deprecate and replace non-owl t-raw #70004
Closed
+889
−970
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
xmo-odoo
force-pushed
the
master-markupsafe-js-xmo
branch
from
6df27e5 to
93ef51b
Compare
xmo-odoo
force-pushed
the
master-markupsafe-js-xmo
branch
from
93ef51b to
2d6ac19
Compare
xmo-odoo
force-pushed
the
master-markupsafe-js-xmo
branch
8 times, most recently
from
1904af8 to
d5c21de
Compare
xmo-odoo
force-pushed
the
master-markupsafe-js-xmo
branch
from
d5c21de to
effb858
Compare
xmo-odoo
force-pushed
the
master-markupsafe-js-xmo
branch
2 times, most recently
from
efde95a to
b769d38
Compare
sdegueldre
reviewed
Jul 5, 2021
xmo-odoo
force-pushed
the
master-markupsafe-js-xmo
branch
from
57ef556 to
27589ff
Compare
xmo-odoo
force-pushed
the
master-markupsafe-js-xmo
branch
4 times, most recently
from
c0e9798 to
7582e74
Compare
Need to check and mark legit uses of HTML as Markup. Also fix some title formattings which are not great (mostly around translations) and de-escape titles which don't need to be escaped anymore.
Convert assets cssContents and jsContents to Markup objects.
`0` should be implicitly markup-safe. Also migrate the two escs to t-out, there's no reason not to.
Mark the message body as safe pretty much as soon as we receive the message from the server: * when receiving message-type notifications * while loading history Also reorder history loading a bit while at it: * convert willStart to async * immediately reverse & wrap history right there, seems unnecessary to wait until willStart since `reverse()` works in-place anyway * when loading messages into the thread, `_.each` seems unnecessary, Array#forEach will do fine
Remove t-raw of alert messages: * Add alert testing to one of the existing tests. * Transform widget data straight in `_fetchWidgetData` so the widget itself only ever sees the "proper" shape of things (to come), this includes the existing parsing and reformatting of `wallet`, as well as the new wrapping of all alerts' `message` in a `Markup`. Note: conditional updating of `alerts` because while the endpoint actual always sets it, test data doesn't necessarily do so (?).
No idea why it's using a t-raw for smileys.
The Activity widget is apparently completely unused now that owl, so remove it, and its template
And convert the `t-esc` in the template to `t-out` while at it. So this was a bit of a bummer initially, the PopoverWidgetField gets a pile of JSON as its value (so from the server), and that pile contains a template name (optionally) and... the template context, basically. For leadDaysPopOver the issue was some of that context is supposed to be rendered HTML to straight inject into the template, marking that as HTML-safe would be a bit of an issue (having the JSON specify which parts of the value it returns are HTML-safe being a bit of a conflict of interest). However turns out the thing is way over-complicated and over-engineered: `lead_days_description` necessarily has a very regular structure owing to being injected as a set of table rows, so the various overrides to `_get_lead_days` just make their own lives complicated by formatting the values they want to return into table rows matching the format of an unrelated template. Instead we can change the signature of `_get_lead_days` so the "description" is a list of values to inject in the table (list of pairs, each pair matching the corresponding columns of the table). The template can then take care of formatting those values into table rows the usual way, removing the need for any injection of raw content. This also makes for better / clearer translation strings.
Mark `display_message` (feedback from payment provider) as markup-safe.
Replace t-raw by t-out when outputting the chatter message. Mark message body as safe during preprocessing: messages from the portal chatter are automatically escaped when posted (`/mail/chatter_post` assumes the message body is plaintext, and escapes it before formatting it based on newlines, maybe one day it'll accept markdown) while messages from the backend chatter are "HTML-light" and should be sanitized.
Using `sale_product_matrix`'s tour as `product_matrix` doesn't actually have any test for this. Remove t-raw from the product_matrix(.extra_price) template, by moving the formatting to the client side (unclear why it was done by the server in the first place). Also fixes a bug where a negative price_extra would have *two* `-` signs: one before the currency, and one after the currency: the price being formatted should be abs'd to avoid a negation being generated by the monetary formatting. Also uses non-breaking spaces everywhere where the server-side formatting mixed breaking and non-breaking spaces. Keeps from the original the peculiarity that the extra price should always have a sign positioned before prefix currency signs. Also changes the calling conventions of `product_matrix.extra_price`: instead of being called with a formatted `price` it's now called with the entire cell object.
Update `_deletePage` to mark `.text` as markup-safe, and modernize the code base: the outer promise seems useless, and so does the `cancel_callback` of the dialog.
Mark the fragments as markup-safe after fetching them. Also make _render private (don't see any reason for it to be publicly accessible), and avoid unnecessary intermediate enc/dec in it.
* direct product attributes seem to not be necessary at all, `list_price` and `price` are just numbers * mark `description_sale` as markup-safe at load * mark ribbon.html as markup-safe at load * mark embed code value as safe on rendering
Replace the t-raws used for the end-of-quiz message by t-outs. This requires marking the two subfields of `quiz/submit` as Markup, rewrite the method in a more modern style while at it. Also update the tour to ensure that the message is indeed inserted as markup, reset the users' karma to a known value to ensure the motivational message is predictable.
Not tested. I probably really need to add that replace() override
Remove t-raw and mark website_description as markup-safe right after fetching it. Also update `_fetchSponsor` to use async/await, and remove seemingly unnecessary empty div in template, and convert `<span>` to `<div>` as the contents is arbitrary and thus could be a block.
xmo-odoo
force-pushed
the
master-markupsafe-js-xmo
branch
from
d387e57 to
11da0fd
Compare
|
@robodoo r+ |
|
Linked pull request(s) odoo/enterprise#18005 not ready. Linked PRs are not staged until all of them are ready. |
|
Staging failed: ci/runbot on a49764dbe69a0eb06162574f5ae76c26a8984f7d (view more at http://runbot.odoo.com/runbot/build/9009395) |
|
@robodoo retry |
robodoo
added a commit
that referenced
this pull request
Jul 20, 2021
non-owl JS-side of #68072: * deprecates `t-raw` * adds a `t-out` which `t-esc` aliases and which "does the right thing" * introduces a `Markup` function / object which is considered markup-safe by t-out (thus left unescaped) - unlike the python version, most of the override hooks (e.g. concatenation, formatting, ...) are not available in JS, we might eventually want to override some of the action methods (e.g. replace) but so far the needs seemed pretty limited - and javascript pretty strongly differentiates between a string (primitive) and a String (object), the latter being what Markup gets - so various constructs which can return a Markup in Python can't really afford to in JS: I tried with QWeb and e.g. jquery *really* does not deal well with non-primitive strings, as a result `_.escape` is shimmed to understand `Markup` objects but will not return markup objects when escaping strings - `sprintf` (the one from web.utils) was updated for Markup-awareness for convenience though - and `Markup` can be used as a template tag, in which case it will automatically escape the substitutions - removed `messageIsHtml` from the non-owl notifications, replaced by the message being a markup object - updated Dialog to work the same way (it did not even have a flag), removed explicit escaping from most of the callsites (only found one where we actually leveraged dialog titles being markup) - modified the kanban view so HTML non-raw values automatically get wrapped in Markup Also moved some formatting from the server to the client, either removing the need to inject markup entirely or making the use of `Markup` much cleaner than just "mark whatever the server returned as safe". There are a few things I'm not entirely sure about e.g. whether even using markup is necessary for `formatMonetary`, doesn't the "unicode" NBSP work fine in HTML? Though we may need to keep `forceString` to ensure something like ascii-compatibility. closes #70004 Related: odoo/enterprise#18005 Signed-off-by: Xavier Morel (xmo) <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
non-owl JS-side of #68072:
t-rawt-outwhicht-escaliases and which "does the right thing"Markupfunction / object which is considered markup-safe by t-out (thus left unescaped)_.escapeis shimmed to understandMarkupobjects but will not return markup objects when escaping stringssprintf(the one from web.utils) was updated for Markup-awareness for convenience thoughMarkupcan be used as a template tag, in which case it will automatically escape the substitutionsmessageIsHtmlfrom the non-owl notifications, replaced by the message being a markup objectAlso moved some formatting from the server to the client, either removing the need to inject markup entirely or making the use of
Markupmuch cleaner than just "mark whatever the server returned as safe".There are a few things I'm not entirely sure about e.g. whether even using markup is necessary for
formatMonetary, doesn't the "unicode" NBSP work fine in HTML? Though we may need to keepforceStringto ensure something like ascii-compatibility.