Vojtech Szocs has posted comments on this change. Change subject: webadmin: unescape cell tooltips string values ......................................................................
Patch Set 2: Code-Review+2 This patch looks good. 1. What about TextCellWithTooltip? The getTooltip(String) method could be like this: SafeHtml value = getEscapedValue((title != null) ? title : value); // today we return value.asString() return new HTML(value.asString()).getText(); // what we could return instead > btw, looking forward, we can adopt the concept of HTML based tool-tips > instead; i.e. something similar to: SafeHtmlWithSafeHtmlTooltipColumn +1 However, there are some things to consider: * SafeHtmlCellWithTooltip + TextCellWithTooltip are generally used for table content cells * SafeHtmlCellWithTooltip is also used as table header cell -- see ColumnResizeCellTable.createHeader, which always uses SafeHtmlCellWithTooltip (directly in createSafeHtmlHeader or indirectly via ResizableHeader which uses SafeHtmlCellWithTooltip as its cell) Maybe we could create dedicated SafeHtml(Header)CellWithTooltip implementation for table header purposes, i.e. if we want to customize view/behavior for header tooltips vs. content tooltips. Anyway, regardless of cell context (header or content cell), tooltip value should be always unescaped. -- To view, visit http://gerrit.ovirt.org/19018 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2da6812694737c212352afda48fa9a50e97f8d60 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alexander Wels <[email protected]> Gerrit-Reviewer: Alexander Wels <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Einav Cohen <[email protected]> Gerrit-Reviewer: Vojtech Szocs <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
