Skip to content

Fix missing html formatting in Doc_Viewer#49326

Merged
timroes merged 5 commits intoelastic:masterfrom
kertal:kertal-pr-2019-10-25-fix-html-fieldformats-in-react
Oct 28, 2019
Merged

Fix missing html formatting in Doc_Viewer#49326
timroes merged 5 commits intoelastic:masterfrom
kertal:kertal-pr-2019-10-25-fix-html-fieldformats-in-react

Conversation

@kertal
Copy link
Copy Markdown
Member

@kertal kertal commented Oct 25, 2019

Summary

After the refactoring of the DocViewer Table layout to React (#43240), HTML produced by Kibana's Field Formatters was displayed as text, instead of e.g. rendering a Link. This PR restores HTML formatted fields, so e.g. Links provided by the url formatter are clickable again. The HTML provided by the formatters is escaped. This is a interim solution until #48728 will be ready

image

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

- [ ] This was checked for cross-browser compatibility, including a check against IE11
- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
- [ ] Documentation was added for features that require explanation or tutorials
- [ ] Unit or functional tests were updated or added to match the most common scenarios
- [ ] This was checked for keyboard-only and screenreader accessibility

For maintainers

- [ ] This was checked for breaking API changes and was labeled appropriately
- [ ] This includes a feature addition or change that requires a release note and was labeled appropriately

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@kertal kertal self-assigned this Oct 28, 2019
@kertal kertal marked this pull request as ready for review October 28, 2019 13:12
Copy link
Copy Markdown
Member Author

@kertal kertal left a comment

Choose a reason for hiding this comment

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

1 main change (added comment), lot's of redundant code

const isCollapsible = typeof value === 'string' && value.length > COLLAPSE_LINE_LENGTH;
const value = trimAngularSpan(String(formatted[field]));

const isCollapsible = value.length > COLLAPSE_LINE_LENGTH;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is the main change, by using the value, formatted by our field formatters, without any component internal modification, it's safe to use dangerouslySetInnerHTML later on. And since this internal formatter isn't used anymore, lot's of helper + test code can be removed.

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@kertal kertal added the Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// label Oct 28, 2019
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@kertal kertal added the Feature:Discover Discover Application label Oct 28, 2019
return typeof valueFormatted === 'string' ? convertAngularHtml(valueFormatted) : String(value);
}
export function trimAngularSpan(text: string): string {
return text.replace('<span ng-non-bindable>', '').replace('</span>', '');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just to be save, can we make sure via regex we're stripping here from the beginning and respectively end of the string?

Copy link
Copy Markdown
Contributor

@timroes timroes left a comment

Choose a reason for hiding this comment

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

Code LGTM, tested on Chrome Linux, works as expected. Tried that XSS injection doesn't work.

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@timroes timroes merged commit df2a996 into elastic:master Oct 28, 2019
timroes pushed a commit to timroes/kibana that referenced this pull request Oct 28, 2019
* Add detection if a value has been formatted, conditional rendering

* Use markup by formatters, it's escaped for dangerouslySetInnerHTML

* Enable dangerouslySetInnerHTML for displaying values

* Use regex for replace
timroes pushed a commit to timroes/kibana that referenced this pull request Oct 28, 2019
* Add detection if a value has been formatted, conditional rendering

* Use markup by formatters, it's escaped for dangerouslySetInnerHTML

* Enable dangerouslySetInnerHTML for displaying values

* Use regex for replace
timroes pushed a commit to timroes/kibana that referenced this pull request Oct 28, 2019
* Add detection if a value has been formatted, conditional rendering

* Use markup by formatters, it's escaped for dangerouslySetInnerHTML

* Enable dangerouslySetInnerHTML for displaying values

* Use regex for replace
timroes pushed a commit that referenced this pull request Oct 28, 2019
* Add detection if a value has been formatted, conditional rendering

* Use markup by formatters, it's escaped for dangerouslySetInnerHTML

* Enable dangerouslySetInnerHTML for displaying values

* Use regex for replace
timroes pushed a commit that referenced this pull request Oct 28, 2019
* Add detection if a value has been formatted, conditional rendering

* Use markup by formatters, it's escaped for dangerouslySetInnerHTML

* Enable dangerouslySetInnerHTML for displaying values

* Use regex for replace
timroes pushed a commit that referenced this pull request Oct 29, 2019
* Add detection if a value has been formatted, conditional rendering

* Use markup by formatters, it's escaped for dangerouslySetInnerHTML

* Enable dangerouslySetInnerHTML for displaying values

* Use regex for replace
@alexwizp alexwizp mentioned this pull request Oct 30, 2019
7 tasks
@kertal kertal deleted the kertal-pr-2019-10-25-fix-html-fieldformats-in-react branch November 27, 2019 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blocker Feature:Discover Discover Application release_note:fix Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.4.2 v7.5.0 v7.6.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants