This repository has been archived by the owner on Jun 26, 2020. It is now read-only.
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.
Provide support for pasting lists from google docs #72
Provide support for pasting lists from google docs #72
Changes from 30 commits
173e44d
7c4f7dc
1111bea
11cbe07
348f3b6
419ff3d
3eda19c
4a566b2
5bf77e7
f6ee361
2fb7ddb
e8e12c9
6c9f2e9
975992e
cfb2a35
95c024f
3806a55
16ca74f
4aa476a
70495e8
68d155e
b8eaff4
861ba50
a6fa897
40c88a2
c9b208f
85c8764
4a1dd43
383b387
1a95ece
676730c
d2da715
5513c92
3d467da
0a56b5e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I wonder if we need GenericNormalizer. It duplicates fixes for GD normalizer. It probably might also duplicate MSWord normalizer.
Right now I don't have a clear idea of how to deal with those fixes - they might be helpfull generally speaking one can copy such weird list indentation from various sources. But OTOH such argument is also valid for every other filter.
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.
I'm thinking about removing this ATM but I'll give it another thought.
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 are the details and comments about why it's important:
#72 (comment)
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.
Does it happen only on Safari? If so I'd still ad some env checking to be sure that this normalizer doesn't fire up too often. @mlewand WDYT? OTOH this normalizer will always fix pasted lists (even from other sources) so it may be beneficial.
I see two routes:
SafariListNormalizer
NestedListNormalizer
and maybe add a check if pasted content has broken list (AFAICSul></ul
,/p></li
and/li><ul
could be checked - withul|ol
variants check) - but the check idea is something to give another though if it makes sense.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.
I would not stick to "List" name here. As content is unrecognizable for any filter.
Google Docs for some specific contents (with tables) under Safari doesn't provide any string which could provide us info, that content came from Google Docs. It's just regular broken HTML.
So the same situation will repeat for heading filters or any other further filters. Where we still won't be knowing that content came from Google Docs.
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.
So in such cases better could be
SafariNormalizer
however this might be a little bit confusing, as there will be content from Safari that sometimes will activateSafariNormalizer
and sometimesGoogleDocsNormalizer
We can prioritize it, but then it will be an exact copy of GoogleDocs normalizer and in such case, in my opinion, better will be making a change in
isActive()
method of google normalizer: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.
@jodator @msamsel I took a peek at it and I see that:
Safari problem described in Provide support for pasting lists from google docs #72 (comment) happens when table is first, but does not happen if there's anything preceding it, which makes the bug that much less common (safari * edge case = very rare case).
First of all it does look like upstream issue, I suspected Safari at first, but given that id appears if table is not the first item it sounds it's more of a Google Docs bug. Have we checked if this issue can be reported and tracked?
I don't feel good with exposing this filter as a generic feature if we don't have any need for it (no requests, no real use cases).
Since still pretty much all the cases will be handled on safari except this particular oddity, let's wait for some real life cases where it will be useful. Until then let's extract mentioned Safari problem as a separate issue and put it to backlog.
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.
@jodator I removed this normalizer.
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.
I extracted this case to separate issue: #75
Large diffs are not rendered by default.
Large diffs are not rendered by default.