Skip to content
This repository was archived by the owner on Feb 16, 2026. It is now read-only.

13.0 web widget html markdown - #4

Closed
gfcapalbo wants to merge 15 commits into
sunflowerit:13.0from
gfcapalbo:13.0-web_widget_html_markdown
Closed

gfcapalbo wants to merge 15 commits into
sunflowerit:13.0from
gfcapalbo:13.0-web_widget_html_markdown

Conversation

@gfcapalbo

Copy link
Copy Markdown

Widget to edit in markdown or HTML, saves always in HTML.

@gfcapalbo
gfcapalbo force-pushed the 13.0-web_widget_html_markdown branch from 91c34b5 to b8a8568 Compare February 8, 2022 18:01
@KKamaa
KKamaa requested a review from thomaspaulb February 9, 2022 14:22
@gfcapalbo
gfcapalbo force-pushed the 13.0-web_widget_html_markdown branch 2 times, most recently from 0da4d2a to 12cb86e Compare February 15, 2022 10:17
@KKamaa

KKamaa commented Feb 15, 2022

Copy link
Copy Markdown

so I used the below markdown text for test:

## TABLE 1

| Syntax | Description |
| --- | ----------- |
| Header | Title |
| Paragraph | Text |

## TABLE 2

| Syntax      | Description |
| ----------- | ----------- |
| Header      | Title       |
| Paragraph   | Text        |

## TABLE 3
| Syntax      | Description | Test Text     |
| :---        |    :----:   |          ---: |
| Header      | Title       | Here's this   |
| Paragraph   | Text        | And more      |

{
"firstName": "John",
"lastName": "Smith",
"age": 25
}


Here's a simple footnote,[^1] and here's a longer one.[^bignote]

[^1]: This is the first footnote.

[^bignote]: Here's one with multiple paragraphs and code.

    Indent paragraphs to include them in the footnote.

    `{ my code }`

    Add as many paragraphs as you like.

First Term
- This is the definition of the first term.

Second Term
- This is one definition of the second term.
- This is another definition of the second term.

~~The world is flat.~~ We now know that the world is round.

- [x] Write the press release
- [ ] Update the website
- [ ] Contact the media

Gone camping! :tent: Be back soon.

That is so funny! :joy:


+ One
+ Two
+ Three
    - Nested One
    - Nested Two

+ One
- Two
* Three

This is a guide on Markdown [Markdown][1].

[1]: http://en.wikipedia.org/wiki/Markdown        "Markdown"

This is an [example link](http://example.com/ "With a Title").

This is an [example link](http://example.com/).

Forcing a line-break\s\s
Next line in the list

![alt text](http://path/to/img.jpg "Title")

* * *

***

*****

- - -

---------------------------------------

# First-level heading

#### Fourth-level heading

This is very heavily **emphasized** __text__.

This is *emphasized* _text_.

Markdown is a `<em>text-to-html</em>` conversion tool for writers.

> ## Blockquoted header
>
> This is blockquoted text.
>
> This is a second paragraph within the blockquoted text.


So if you do conversion from markdown to html but through saves you will notice its losing data, due to showdown conversion from html to showdown again.

Links to test samples:

@KKamaa

KKamaa commented Feb 15, 2022

Copy link
Copy Markdown

There is an issue with italics they are being escaped by _\italics_ if you add underscore, showdown prefers using asterisks ** to represent italics. Think this can be by passed when a user adds whichever the case _ or *, it just treats it as italics for all. Had a similar issue, so I override the italics button under $.fn.markdown.defaults.buttons as shown below:

$.fn.markdown.defaults.buttons[0].forEach(function(group) {
                group.data.forEach(function(button) {
                    if (button.name === "cmdItalic") cmdItalic = button;
                });
            });

            // Override `cmdItalic` button replace use of __ to favour **;
            if (cmdItalic) {
                cmdItalic.callback = function(e) {
                    // Give/remove * surround the selection
                    var chunk = null;
                    var cursor = null;
                    var selected = e.getSelection();
                    var content = e.getContent();
                    if (selected.length === 0) {
                        // Give extra word
                        chunk = e.__localize("emphasized text");
                    } else {
                        chunk = selected.text;
                    }
                    // Transform selection and set the cursor into chunked text
                    if (
                        content.substr(selected.start - 1, 1) === "_" &&
                        content.substr(selected.end, 1) === "_"
                    ) {
                        e.setSelection(selected.start - 1, selected.end + 1);
                        e.replaceSelection(chunk);
                        cursor = selected.start - 1;
                    } else {
                        e.replaceSelection("*" + chunk + "*");
                        cursor = selected.start + 1;
                    }
                    // Set the cursor
                    e.setSelection(cursor, cursor + chunk.length);
                };
            }
        },

@KKamaa

KKamaa commented Feb 15, 2022

Copy link
Copy Markdown

Issue about losing of data during conversion has to do with how html renders markdown,or rather how it interprets markdown syntax from the conversion from html -> markdown syntax. Normally, it might not affect small conversions, but in cases where we use a lot of complex markdown it is bound to happen. Maybe we can store the original mark down text and only do html for rendering only instead of bidirectional, which loses data? Bidirectional conversion doesn't seem consistent all through for same content.

@gfcapalbo
gfcapalbo force-pushed the 13.0-web_widget_html_markdown branch from 7030824 to 12cb86e Compare February 15, 2022 12:23
@gfcapalbo

Copy link
Copy Markdown
Author

@thomaspaulb worked perfectly. with some nice feedback for the user.
Unfortunately just at the end i get a bug. On save the script tag is not saved.
tag

After save it is gone.
I tried debugging , but it got late and i can't think right anymore. I may revisit this bug
the tag works perfectly when switching back and forth, without editing HTML , before saving.
Also if I manage to make the tag survive save, the code is in place to make widget startup in MD mode.

I will try a "stupid" re-injection after save now.

Maybe you have an intuition here.

@gfcapalbo

Copy link
Copy Markdown
Author

@thomaspaulb after sending you a message i found the problem. please no not investigate
will commit soon 100% final working version

@gfcapalbo

Copy link
Copy Markdown
Author

@thomaspaulb the bug i found had to be fixed, but the removal of script on save remains.
I suspect it is caused by the sanitize functions of the HTML field itself.

@gfcapalbo

Copy link
Copy Markdown
Author

I tried to remove default sanitize from HTML field. It does not work.
Tried to debug the super of commitChanges to see where did it purge our tag.
I just got lost in a sea of promises.
Can't figure this out.

I could just save the tag, and then impose a XML-RPC call with a write on the field that forces the tag to be appended, but it seems like a bad solution. there must be a way.

@gfcapalbo

Copy link
Copy Markdown
Author

We are certain the missing tag is not do to the a cleanup done in the rendering phase, because it is committed to DB without the tag:
wwmd=# select description from project_task where id=19;
description

                       +
                       +
                       +

+
+
+

ass
+ fadafsfassfafsdaafsfas

+ +

so it's something in the save function.

If i go in HTML-source mode i see the editor preserves COMMENTS in the source, i may be able to do a trick there.

@gfcapalbo

Copy link
Copy Markdown
Author

I tried everything, wrapping in it a comment. nothing.
When using Html-Code editor (the </> icon ) and adding manually

<script> </script> and then saving, the editor eliminates that too.

so it;s something in the editor itself that wipes out <script> tags.

technically the does not wipe out comments , maybe we can do something with that.

Worse case scenario: If I can't figure it out i can support this widget with a small model , with a text field that contains the
"tag"
'field it belongs to'
'res_id'

on commitChanges i would save the tag to my support model, and on first instanciation of WYSIWYG read i would check if a tag exists and fetch it...

@gfcapalbo

Copy link
Copy Markdown
Author

The best solution is to plugin to summernote and extend it to help with this behaviour,

I have suspicions here:
https://github.com/OCA/OCB/blob/13.0/addons/web_editor/static/src/js/editor/editor.js#L165-L181

@thomaspaulb

Copy link
Copy Markdown
Member

@gfcapalbo Call it a weekend!! :-)

My gut feeling says we should be able to just circumvent this behaviour of summernote, by not having the markdown pass to summernote at all - we only need it for the markdown widget anyway. We re-add it upon saving in markdown mode.

Comment thread web_widget_html_markdown/__manifest__.py
Comment thread web_widget_html_markdown/readme/CREDITS.rst Outdated
Comment thread web_widget_html_markdown/readme/CONTRIBUTORS.rst Outdated
Comment thread web_widget_html_markdown/demo/bootstrap_markdown.xml
Comment thread web_widget_html_markdown/readme/DESCRIPTION.rst Outdated
Comment thread web_widget_html_markdown/static/src/js/web_widget_html_markdown.js Outdated
Comment thread web_widget_html_markdown/static/src/js/web_widget_html_markdown.js
});
} else {
// yes repeated block of code, the one above needs to be executed async,
// after the wyswyg has rendered

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MMMm okaaaay, but i'm not getting why save needs to be called if mode is not "edit" ? Perhaps this needs to be else if (self.mode == 'edit')

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Putting a pin in this. not closing

Comment thread web_widget_html_markdown/static/src/js/web_widget_html_markdown.js Outdated
@gfcapalbo
gfcapalbo force-pushed the 13.0-web_widget_html_markdown branch from 8d4fdfc to 588db8b Compare March 28, 2022 14:24
@gfcapalbo

Copy link
Copy Markdown
Author

@thomaspaulb
Succeded in passing to write what we want


2022-03-30 09:33:54,491 37494 DEBUG wwmd odoo.api: call project.task(13,).write({'description': '\n\n\n<meta charset="utf-8">\n\n\n<ul>\n<li><strong>a99</strong></li>\n</ul>\n\n'}) 
2022-03-30 09:33:54,499 37494 INFO wwmd odoo.addons.web_widget_html_markdown.models.base: BEFORE Processing {'description': '\n\n\n<meta charset="utf-8">\n\n\n<ul>\n<li><strong>a99</strong></li>\n</ul>\n\n'} 
2022-03-30 09:34:12,000 37494 INFO wwmd odoo.addons.web_widget_html_markdown.models.base: AFTER Processing {'description': '\n\n\n<meta charset="utf-8">\n\n\n<ul>\n<li><strong>a999</strong></li>\n</ul>\n\n<script class="web_widget_html_markdown_source" type="text/plain"> - **a999**\n\n</script>'}

I inherit base, and reprocess.
Unfortunately in DB:

wwmd=# select description from project_task  where id=13
;
          description           
--------------------------------
 <ul>                          +
 <li><strong>a999</strong></li>+
 </ul>                         +
                               +
 
(1 riga)

wwmd=# 

THis means that the cleanup is done in HTML FIELD in the backend too?
Digging deeper.

@gfcapalbo

Copy link
Copy Markdown
Author

Interesting ideas here:
https://github.com/odoo/odoo/blob/13.0/odoo/tools/mail.py#L180

Obviously there are security issues for <script> usage and loading....

Easy Idea:
I could wrap <script > in something else on write , and unwrap it on read, in order to bypass the "sanitization" process

@gfcapalbo

Copy link
Copy Markdown
Author

HTML field uses html_sanitize from /tools https://github.com/odoo/odoo/blob/13.0/odoo/fields.py#L23

there you go.
No need to inherit fields. I just need to inherit tools html_sanitize and extend it, to skip removing scripts when we have our context key. That is probably why the JS part also "sanitized" not the editor. Basically any read on this field will call html_sanitize. so Json_calls etc... everything will pass through that "filter".
There are good reasons to do that, but i think we can make an exception.

So i have to monkey patch html_sanitize
https://github.com/odoo/odoo/blob/13.0/odoo/tools/mail.py#L180

It fooled me because it is in mail.

@gfcapalbo

Copy link
Copy Markdown
Author

@thomaspaulb It is incredible.
textarea, "allowed" tag has the same behaviour:


188133 2022-03-30 13:23:47,820 83654 INFO wwmd odoo.addons.web_widget_html_markdown.models.base: BEFORE Processing {'description': '<ul>\n<li><strong>a999ffffffff addada</strong>           aasdsssssss</li>\n</ul       >\n\n'}
188134 2022-03-30 13:23:49,852 83654 DEBUG ? odoo.service.server: cron1 polling for jobs       
188135 2022-03-30 13:24:01,620 83654 DEBUG wwmd odoo.modules.registry: Multiprocess signaling check: [Registry - 5 -> 5] [Cache - 39 -> 39]
188136 2022-03-30 13:24:06,216 83654 INFO wwmd odoo.addons.web_widget_html_markdown.models.base: AFTER Processing {'description': '\n\n\n<meta charset="utf-8">\n\n\n<ul>\n<li><strong>a999ffffffff addada</strong>        aasdsssssss<br>\nsasa</li>\n</ul>\n\n<textarea style="display:none;" class="web_widget_html_markdown_source" type="text/plain"> - **a999ffffffff addada** aasdsssssss\n\nsasa</textarea>'}

but then in DB:

wwmd=# select description from project_task  where id=13
;
                       description                        
----------------------------------------------------------
 <ul>                                                    +
 <li><strong>a999ffffffff addada</strong> aasdsssssss<br>+
 sasa</li>                                               +
 </ul>                                                   +
                                                         +

i send the correct HTML to write function, but it is still wiped out.
Investigating the html_sanitize processing for the html field.

@gfcapalbo

Copy link
Copy Markdown
Author

just got tag in DB. finishing up.

@gfcapalbo

Copy link
Copy Markdown
Author

@thomaspaulb done:

  • tag working
  • linted with esLint
  • starts in markdown if tag present (tested)
  • fixed errors on feedback.
  • verified tag is in the db
  • warning comment in tag for advanced users

the tag is an invisible p. verifying it works with textarea (had problems with it, but may be due to my mistake.)
also the 'base' inheritance + context is perhaps USESLESS NOW. unfortunately.

will verify that too and see if it is needed,

@gfcapalbo

Copy link
Copy Markdown
Author

Small corner-case error where it does not save data. fixing.

@thomaspaulb

Copy link
Copy Markdown
Member

@gfcapalbo I have a shitty request, but as we originally planned to migrate sunflower to 13.0, this is in 13.0.

But now we plan to just migrate our data into Therp's 14.0, and so we need it in 14.0.

Could you forward-port and open a new MR? Then we can test it as part of helptest14, if Danny includes it in the migration.

@gfcapalbo

Copy link
Copy Markdown
Author

@thomaspaulb last bug?

usecase:
1.switch HTML=>MD
2.edit MD>
3. switch to HTML

 --- it doesn;t save.

why:

the editor does not save if no changes have been made. Because no onchanges have never been called in this "new" WYSIWYG object:

   if (!(options && options.forceChange) && this._isSameValue(value)) {
            return Promise.resolve();
        }

this would make: saving from MD fail. saving from HTML with no edit fail. the most important cases for our tag!!!!

but interestengly enough we have "forceChange" , it works.
It took a while to understand whats going on . in this widget we destroy and recreate WYSIWYG instances every time.
ALL works now.

FEATURES:
- it saves in any possible situation (tested 45minutes)
- feedback.
- starts in markdown if the tag is there
- fetches markdown tag if available when loading markdown
- multiple widgets on same page all work
- BUG solved: switcher now has right location if we start on markdown.

@gfcapalbo
gfcapalbo force-pushed the 13.0-web_widget_html_markdown branch from 1b08872 to 726b87a Compare March 31, 2022 17:29
@gfcapalbo

Copy link
Copy Markdown
Author

@thomaspaulb please note this commit a good improvement:
726b87a

I am very happy with this improvement. works very well.

@gfcapalbo

Copy link
Copy Markdown
Author

@thomaspaulb lastly, the tag in context thing is not needed anymore.

@gfcapalbo

Copy link
Copy Markdown
Author

@thomaspaulb
I found a bug, while porting to v14.
in general it is very solid and tested a LOT.
but, when you start with an empty content area, if you switch to MD it raises error. I must manage this case.
will push fixes to v13 and v14.

@thomaspaulb

Copy link
Copy Markdown
Member

@gfcapalbo Closing this in favour of the 14.0 MR, which is the one we'll focus on making Therp- and then OCA-ready.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants