Alexander Wels has posted comments on this change.

Change subject: 4. webadmin: Changing LoginPopup to present clickable password 
URL change
......................................................................


Patch Set 6:

(2 comments)

http://gerrit.ovirt.org/#/c/23622/6/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/presenter/AbstractLoginPopupPresenterWidget.java
File 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/presenter/AbstractLoginPopupPresenterWidget.java:

Line 166:                 String url = message.substring(urlIndex);
Line 167:                 StringBuilder htmlPart = new StringBuilder();
Line 168:                 htmlPart.append(beforeURL)
Line 169:                     .append("<a href=\"").append(url).append("\" 
target=\"_blank\">").append(url).append("</a>"); //$NON-NLS-1$ //$NON-NLS-2$ 
//$NON-NLS-3$
Line 170:                 message = htmlPart.toString();
Why aren't we using SafeHtmlBuilder or an Anchor to build the URL? It is never 
a good plan to just build HTML from an unknown source (even though in this case 
we know the source, but potentially the message can come from anywhere).
Line 171:             }
Line 172:         }
Line 173:         getView().setErrorMessageHtml(message);
Line 174:     }


http://gerrit.ovirt.org/#/c/23622/6/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/view/AbstractLoginPopupView.java
File 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/view/AbstractLoginPopupView.java:

Line 111:         asWidget().setKeyPressHandler(keyPressHandler);
Line 112:     }
Line 113: 
Line 114:     protected void setErrorMessageLabel(Label errorMessage, String 
text) {
Line 115:             errorMessage.getElement().setInnerHTML(text);
If we are going to put arbitrary HTML in the MOTD, wouldn't it make more sense 
to use the HTML object instead of the Label object? that way you don't have to 
do things like Label.getElement().setInnerHTML();
Line 116:     }
Line 117: 


-- 
To view, visit http://gerrit.ovirt.org/23622
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I502cfcad23c3500f1e87e3f45657cf61e9c36d69
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.4
Gerrit-Owner: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Greg Sheremeta <[email protected]>
Gerrit-Reviewer: Itamar Heim <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Leonardo Bianconi <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to