Vojtech Szocs has posted comments on this change.
Change subject: userportal, webadmin: workaround for Firefox favicon bug
......................................................................
Patch Set 2:
(4 comments)
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/FaviconRefreshUtil.java
Line 14: * seems to occur too late there.
Line 15: */
Line 16: public static void refreshFavicon() {
Line 17: Node favicon = DOM.getElementById("id-link-favicon");
//$NON-NLS-1$
Line 18: Node parent = favicon.getParentNode();
To guard against potential changes in GWT host page, I'd suggest putting a null
check here, just to be on safe side:
if (favicon != null) {
...
}
Line 19: favicon.removeFromParent();
Line 20: parent.appendChild(favicon);
Line 21: }
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/system/BaseApplicationInit.java
Line 157: * Simply detach and re-attach the favicon link tag on hashchange.
Line 158: */
Line 159: private void registerHistoryChangeHandler() {
Line 160: if ("firefox".equalsIgnoreCase(clientAgentType.getBrowser()))
{ //$NON-NLS-1$
Line 161: History.addValueChangeHandler(new
ValueChangeHandler<String>() {
In GWTP framework, place navigation is driven by GWTP PlaceManager whose
default implementation (PlaceManagerImpl) uses GWT History mechanism.
Instead of attaching value change handler to History directly, we could just
override relevant method in app-specific PlaceManager, for example:
public abstract class ApplicationPlaceManager ... {
...
@Override
public void onValueChange(ValueChangeEvent<String> event) {
if ("firefox".equalsIgnoreCase(clientAgentType.browser) {
FaviconRefreshUtil.refreshFavicon();
}
}
...
}
This way, we don't need any change in BaseApplicationInit and also no need for
extra stuff like overriding LoginSectionPresenter.onReset method.
Greg, can you please test this solution?
Line 162:
Line 163: @Override
Line 164: public void onValueChange(ValueChangeEvent<String>
event) {
Line 165: FaviconRefreshUtil.refreshFavicon();
....................................................
File
frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/section/login/presenter/LoginSectionPresenter.java
Line 46: @Override
Line 47: protected void onReset() {
Line 48: super.onReset();
Line 49:
Line 50: // work around Firefox favicon bug that appears on login screen
Please see my comment in BaseApplicationInit.java - this change might not be
necessary at all.
Line 51: if ("firefox".equalsIgnoreCase(clientAgentType.getBrowser()))
{ //$NON-NLS-1$
Line 52: FaviconRefreshUtil.refreshFavicon();
Line 53: }
Line 54:
....................................................
File
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/login/presenter/LoginSectionPresenter.java
Line 46: @Override
Line 47: protected void onReset() {
Line 48: super.onReset();
Line 49:
Line 50: // work around Firefox favicon bug that appears on login screen
Please see my comment in BaseApplicationInit.java - this change might not be
necessary at all.
Line 51: if ("firefox".equalsIgnoreCase(clientAgentType.getBrowser()))
{ //$NON-NLS-1$
Line 52: FaviconRefreshUtil.refreshFavicon();
Line 53: }
Line 54:
--
To view, visit http://gerrit.ovirt.org/20798
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia4b0d8a0176eb9299aec01ae6772297409ef3108
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Greg Sheremeta <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Greg Sheremeta <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[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
