Cover block upload completion processor #11541
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| package org.wordpress.android.ui.posts.mediauploadcompletionprocessors; | ||
|
|
||
| import com.google.gson.JsonObject; | ||
|
|
||
| import org.jsoup.nodes.Document; | ||
| import org.jsoup.nodes.Element; | ||
| import org.wordpress.android.util.helpers.MediaFile; | ||
|
|
||
| import java.util.regex.Matcher; | ||
| import java.util.regex.Pattern; | ||
|
|
||
| public class CoverBlockProcessor extends BlockProcessor { | ||
| /** | ||
| * Template pattern used to match and splice cover inner blocks | ||
| */ | ||
| private static final Pattern PATTERN_COVER_INNER = Pattern.compile(new StringBuilder() | ||
| .append("(^.*?<div class=\"wp-block-cover__inner-container\">\\s*)") | ||
| .append("(.*)") // inner block contents | ||
| .append("(\\s*</div>\\s*</div>\\s*<!-- /wp:cover -->.*)").toString(), Pattern.DOTALL); | ||
|
There was a problem hiding this comment. oops. I had left a comment here that was wrong. you can take some inspiration from There was a problem hiding this comment. Hi @dmsnell 👋 😃 , thanks for taking a look at this one. I'm using jsoup already, which is really helpful. IIRC, I encountered some minor differences in output in certain scenarios, possible because it parses to an AST and not a parse tree, however, it is probably worth revisiting, as the variations might still be acceptable to pass validation. 👍 There was a problem hiding this comment.
Do you remember what those differences were? Any difference would have to be a bug since we're dealing with HTML/CSS selectors… There was a problem hiding this comment. also it looks like this entire pattern shouldn't be necessary if we process the inner blocks using a spec-compliant parser There was a problem hiding this comment.
Off-hand, I recall that jsoup was adding closing tags, which was problematic during at least one of the iterations toward the current implementation. It's likely this is resolved now, as the implementation no longer processes units of non-self-contained HTML, so probably worth revisiting. To be sure, this was not a bug in jsoup, but rather a limitation in a particular iteration of this implementation, in that it required serialization of the parse tree, while jsoup is designed to serialize abstract trees.
Indeed, using a spec-compliant parser would obviate the need for this, as we'd be able to handle inner blocks an a much more general way, albeit for all block types. |
||
|
|
||
| /** | ||
| * Pattern to match background-image url in cover block html content | ||
| */ | ||
| private static final Pattern PATTERN_BACKGROUND_IMAGE_URL = Pattern.compile( | ||
| "background-image:\\s*url\\([^\\)]+\\)"); | ||
|
|
||
| private final MediaUploadCompletionProcessor mMediaUploadCompletionProcessor; | ||
|
|
||
| public CoverBlockProcessor(String localId, MediaFile mediaFile, | ||
| MediaUploadCompletionProcessor mediaUploadCompletionProcessor) { | ||
| super(localId, mediaFile); | ||
| mMediaUploadCompletionProcessor = mediaUploadCompletionProcessor; | ||
| } | ||
|
|
||
| @Override String processInnerBlock(String block) { | ||
| Matcher innerMatcher = PATTERN_COVER_INNER.matcher(block); | ||
| boolean innerCapturesFound = innerMatcher.find(); | ||
|
|
||
| // process inner contents recursively | ||
| if (innerCapturesFound) { | ||
| String innerProcessed = mMediaUploadCompletionProcessor.processContent(innerMatcher.group(2)); // | ||
| return new StringBuilder() | ||
| .append(innerMatcher.group(1)) | ||
| .append(innerProcessed) | ||
| .append(innerMatcher.group(3)) | ||
| .toString(); | ||
| } | ||
|
|
||
| return block; | ||
| } | ||
|
|
||
| @Override boolean processBlockJsonAttributes(JsonObject jsonAttributes) { | ||
| if (jsonAttributes.get("id").getAsInt() == Integer.parseInt(mLocalId, 10)) { | ||
| jsonAttributes.addProperty("id", Integer.parseInt(mRemoteId, 10)); | ||
| jsonAttributes.addProperty("url", mRemoteUrl); | ||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| @Override boolean processBlockContentDocument(Document document) { | ||
| // select cover block div | ||
| Element targetDiv = document.select(".wp-block-cover").first(); | ||
|
|
||
| // if a match is found, proceed with replacement | ||
| if (targetDiv != null) { | ||
| // replace background-image url in style attribute | ||
| String style = PATTERN_BACKGROUND_IMAGE_URL.matcher(targetDiv.attr("style")) | ||
| .replaceFirst(String.format("background-image:url(%1$s)", mRemoteUrl)); | ||
| targetDiv.attr("style", style); | ||
|
There was a problem hiding this comment. hm…I'm seeing the cover block actually store the background imagine inside the JSON attributes. this will deifnitely update the style, but won't the newly-uploaded image get wiped out after someone loads the post in the editor again and there's a mismatch between what the attributes indicate and what HTML has been generated? we probably need to update both places. There was a problem hiding this comment.
The Json attributes are updated in an earlier step via the abstract method |
||
|
|
||
| // return injected block | ||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
| } | ||
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 there a reason to not make the default processor process inner blocks? It will come surprisingly often that the blocks we care about are nested, for example, when people build with the full-site-editing layout blocks, or the Group block, or the Quote block, or the Gallery block…
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.
For the current implementation of the processors, they are not full-blown parsers, but part of their behavior resembles a parser. Instead, the processors are only concerned with a subset of block types (
image,video, etc.). To hopefully clarify: whenprocessContentencounters a Group block, it does not match on that block, but if that block's inner contents contains a media-containing block, that will match and be processed further (viaprocessBlock), while all content preceeding that match is treated as a raw string. This is a way to produce the desired output without the need to implement any serialization logic for all potential block types.