Fix search clickthrough for HTML events

Switch to using a normal <a href="..."> link for search result
clickthrough. Apart from generally giving a better experience, this means that
it also works on html messages. The problem there was that we were attaching
onClick handlers to <span>s which we were then flattening into HTML with
ReactDOMServer (which meant the onClick handlers were never attached to React's
list of listeners).

To make this work without jumping through React hoops, the highlighter now
returns either a list of strings or a list of nodes, depending on whether we
are dealing with an HTML event or a text one. We therefore have a separate
HtmlHighlighter and TextHighlighter.
pull/21833/head
Richard van der Hoff 2016-02-17 19:50:04 +00:00
parent 38a2a61b38
commit e3feae32e1
6 changed files with 102 additions and 48 deletions

View File

@ -17,7 +17,6 @@ limitations under the License.
'use strict';
var React = require('react');
var ReactDOMServer = require('react-dom/server')
var sanitizeHtml = require('sanitize-html');
var highlight = require('highlight.js');
@ -50,14 +49,23 @@ var sanitizeHtmlParams = {
},
};
class Highlighter {
constructor(html, highlightClass, onHighlightClick) {
this.html = html;
class BaseHighlighter {
constructor(highlightClass, highlightLink) {
this.highlightClass = highlightClass;
this.onHighlightClick = onHighlightClick;
this._key = 0;
this.highlightLink = highlightLink;
}
/**
* apply the highlights to a section of text
*
* @param {string} safeSnippet The snippet of text to apply the highlights
* to.
* @param {string[]} safeHighlights A list of substrings to highlight,
* sorted by descending length.
*
* returns a list of results (strings for HtmlHighligher, react nodes for
* TextHighlighter).
*/
applyHighlights(safeSnippet, safeHighlights) {
var lastOffset = 0;
var offset;
@ -71,10 +79,12 @@ class Highlighter {
nodes = nodes.concat(this._applySubHighlights(subSnippet, safeHighlights));
}
// do highlight
nodes.push(this._createSpan(safeHighlight, true));
// do highlight. use the original string rather than safeHighlight
// to preserve the original casing.
var endOffset = offset + safeHighlight.length;
nodes.push(this._processSnippet(safeSnippet.substring(offset, endOffset), true));
lastOffset = offset + safeHighlight.length;
lastOffset = endOffset;
}
// handle postamble
@ -92,31 +102,64 @@ class Highlighter {
}
else {
// no more highlights to be found, just return the unhighlighted string
return [this._createSpan(safeSnippet, false)];
return [this._processSnippet(safeSnippet, false)];
}
}
}
class HtmlHighlighter extends BaseHighlighter {
/* highlight the given snippet if required
*
* snippet: content of the span; must have been sanitised
* highlight: true to highlight as a search match
*
* returns an HTML string
*/
_processSnippet(snippet, highlight) {
if (!highlight) {
// nothing required here
return snippet;
}
var span = "<span class=\""+this.highlightClass+"\">"
+ snippet + "</span>";
if (this.highlightLink) {
span = "<a href=\""+encodeURI(this.highlightLink)+"\">"
+span+"</a>";
}
return span;
}
}
class TextHighlighter extends BaseHighlighter {
constructor(highlightClass, highlightLink) {
super(highlightClass, highlightLink);
this._key = 0;
}
/* create a <span> node to hold the given content
*
* spanBody: content of the span. If html, must have been sanitised
* snippet: content of the span
* highlight: true to highlight as a search match
*
* returns a React node
*/
_createSpan(spanBody, highlight) {
_processSnippet(snippet, highlight) {
var spanProps = {
key: this._key++,
};
if (highlight) {
spanProps.onClick = this.onHighlightClick;
spanProps.className = this.highlightClass;
}
if (this.html) {
return (<span {...spanProps} dangerouslySetInnerHTML={{ __html: spanBody }} />);
}
else {
return (<span {...spanProps}>{ spanBody }</span>);
var node = <span {...spanProps}>{ snippet }</span>;
if (highlight && this.highlightLink) {
node = <a href={this.highlightLink}>{node}</a>
}
return node;
}
}
@ -128,8 +171,7 @@ module.exports = {
*
* highlights: optional list of words to highlight, ordered by longest word first
*
* opts.onHighlightClick: optional callback function to be called when a
* highlighted word is clicked
* opts.highlightLink: optional href to add to highlights
*/
bodyToHtml: function(content, highlights, opts) {
opts = opts || {};
@ -144,18 +186,13 @@ module.exports = {
// by an attempt to search for 'foobar'. Then again, the search query probably wouldn't work either
try {
if (highlights && highlights.length > 0) {
var highlighter = new Highlighter(isHtml, "mx_EventTile_searchHighlight", opts.onHighlightClick);
var highlighter = new HtmlHighlighter("mx_EventTile_searchHighlight", opts.highlightLink);
var safeHighlights = highlights.map(function(highlight) {
return sanitizeHtml(highlight, sanitizeHtmlParams);
});
// XXX: hacky bodge to temporarily apply a textFilter to the sanitizeHtmlParams structure.
sanitizeHtmlParams.textFilter = function(safeText) {
return highlighter.applyHighlights(safeText, safeHighlights).map(function(span) {
// XXX: rather clunky conversion from the react nodes returned by applyHighlights
// (which need to be nodes for the non-html highlighting case), to convert them
// back into raw HTML given that's what sanitize-html works in terms of.
return ReactDOMServer.renderToString(span);
}).join('');
return highlighter.applyHighlights(safeText, safeHighlights).join('');
};
}
safeBody = sanitizeHtml(content.formatted_body, sanitizeHtmlParams);
@ -167,7 +204,7 @@ module.exports = {
} else {
safeBody = content.body;
if (highlights && highlights.length > 0) {
var highlighter = new Highlighter(isHtml, "mx_EventTile_searchHighlight", opts.onHighlightClick);
var highlighter = new TextHighlighter("mx_EventTile_searchHighlight", opts.highlightLink);
return highlighter.applyHighlights(safeBody, highlights);
}
else {

View File

@ -879,15 +879,6 @@ module.exports = React.createClass({
});
},
_onSearchResultSelected: function(result) {
var event = result.context.getEvent();
dis.dispatch({
action: 'view_room',
room_id: event.getRoomId(),
event_id: event.getId(),
});
},
getSearchResultTiles: function() {
var EventTile = sdk.getComponent('rooms.EventTile');
var SearchResultTile = sdk.getComponent('rooms.SearchResultTile');
@ -948,10 +939,12 @@ module.exports = React.createClass({
}
}
var resultLink = "#/room/"+this.props.roomId+"/"+mxEv.getId();
ret.push(<SearchResultTile key={mxEv.getId()}
searchResult={result}
searchHighlights={this.state.searchHighlights}
onSelect={this._onSearchResultSelected.bind(this, result)}/>);
resultLink={resultLink}/>);
}
return ret;
},

View File

@ -28,6 +28,18 @@ module.exports = React.createClass({
}
},
propTypes: {
/* the MatrixEvent to show */
mxEvent: React.PropTypes.object.isRequired,
/* a list of words to highlight */
highlights: React.PropTypes.array,
/* link URL for the highlights */
highlightLink: React.PropTypes.string,
},
render: function() {
var UnknownMessageTile = sdk.getComponent('messages.UnknownBody');
@ -48,6 +60,6 @@ module.exports = React.createClass({
}
return <TileType mxEvent={this.props.mxEvent} highlights={this.props.highlights}
onHighlightClick={this.props.onHighlightClick} />;
highlightLink={this.props.highlightLink} />;
},
});

View File

@ -28,6 +28,17 @@ linkifyMatrix(linkify);
module.exports = React.createClass({
displayName: 'TextualBody',
propTypes: {
/* the MatrixEvent to show */
mxEvent: React.PropTypes.object.isRequired,
/* a list of words to highlight */
highlights: React.PropTypes.array,
/* link URL for the highlights */
highlightLink: React.PropTypes.string,
},
componentDidMount: function() {
linkifyElement(this.refs.content, linkifyMatrix.options);
@ -46,14 +57,15 @@ module.exports = React.createClass({
shouldComponentUpdate: function(nextProps) {
// exploit that events are immutable :)
return (nextProps.mxEvent.getId() !== this.props.mxEvent.getId() ||
nextProps.highlights !== this.props.highlights);
nextProps.highlights !== this.props.highlights ||
nextProps.highlightLink !== this.props.highlightLink);
},
render: function() {
var mxEvent = this.props.mxEvent;
var content = mxEvent.getContent();
var body = HtmlUtils.bodyToHtml(content, this.props.highlights,
{onHighlightClick: this.props.onHighlightClick});
{highlightLink: this.props.highlightLink});
switch (content.msgtype) {
case "m.emote":

View File

@ -96,8 +96,8 @@ module.exports = React.createClass({
/* a list of words to highlight */
highlights: React.PropTypes.array,
/* a function to be called when the highlight is clicked */
onHighlightClick: React.PropTypes.func,
/* link URL for the highlights */
highlightLink: React.PropTypes.string,
/* is this the focussed event */
isSelectedEvent: React.PropTypes.bool,
@ -313,8 +313,8 @@ module.exports = React.createClass({
{ avatar }
{ sender }
<div className="mx_EventTile_line">
<EventTileType mxEvent={this.props.mxEvent} highlights={this.props.highlights}
onHighlightClick={this.props.onHighlightClick} />
<EventTileType mxEvent={this.props.mxEvent} highlights={this.props.highlights}
highlightLink={this.props.highlightLink}/>
</div>
</div>
);

View File

@ -29,8 +29,8 @@ module.exports = React.createClass({
// a list of strings to be highlighted in the results
searchHighlights: React.PropTypes.array,
// callback to be called when the user selects this result
onSelect: React.PropTypes.func,
// href for the highlights in this result
resultLink: React.PropTypes.string,
},
render: function() {
@ -53,7 +53,7 @@ module.exports = React.createClass({
}
if (EventTile.haveTileForEvent(ev)) {
ret.push(<EventTile key={eventId+"+"+j} mxEvent={ev} contextual={contextual} highlights={highlights}
onHighlightClick={this.props.onSelect}/>)
highlightLink={this.props.resultLink}/>);
}
}
return (