jenkins-bot has submitted this change and it was merged.

Change subject: Make items on notifications flyout behave like links
......................................................................


Make items on notifications flyout behave like links

We want the notifications in the flyout to behave just like links,
including standard middle-click and Ctrl-click behavior. The simplest
way to do that would be to actually make them links - but the area can
contain a few other links, so we can't do that and have to resort to
ugly hacks.

Or do we?

Turns out that while browsers won't accept HTML containing nested <a>
tags[1], such a structure is valid XHTML, and it's possible to create
such structure in HTML mode using DOM manipulation. It works like one
would expect: the entire thing is clickable, but inner <a> tags' hrefs
override outer ones.

Firefox even had a request to make that work[2] which was happily
fulfilled.

Tested the basic case [see below] on Firefox 22, Opera 12, Opera 15
(which uses the Blink engine like Chrome), IE 8 and IE 6 and it works
the same on all of them. Tested the XHTML variant [see below] on all
of the above except for the IEs which don't grok XHTML and it exhibits
the same behavior.

[1] Simple test: $('<div>1<a>2<a>3</a>4</a>5</div>').html() is
    "1<a>2</a><a>3</a>45", not actually "1<a>2<a>3</a>4</a>5" like one
    might expect.
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=331959

----

The test cases used are below. When trying out the XHTML one make sure
that the browser uses application/xhtml+xml MIME type; saving the file
with .xhtml extension should be enough.

XHTML:
  <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" 
"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd";>
  <html xmlns="http://www.w3.org/1999/xhtml";>
  <body>
  <div>1<a href="http://google.com/";>2<a 
href="http://example.com/";>3</a>4</a>5</div>
  </body>
  </html>

HTML:
  <!DOCTYPE html>
  <html>
  <body>
  <script type="text/javascript">
  var div = document.createElement('div');
  var a1 = document.createElement('a');
  a1.href = "http://google.com/";;
  var a2 = document.createElement('a');
  a2.href = "http://example.com/";;

  div.appendChild( document.createTextNode('1') );
  div.appendChild( a1 );
  a1.appendChild( document.createTextNode('2') );
  a1.appendChild( a2 );
  a2.appendChild( document.createTextNode('3') );
  a1.appendChild( document.createTextNode('4') );
  div.appendChild( document.createTextNode('5') );

  document.body.appendChild(div);
  </script>
  </body>
  </html>

----

Bug: 52319
Change-Id: I311eca70f025ce92129c828cd88f96686b7cff72
---
M modules/overlay/ext.echo.overlay.css
M modules/overlay/ext.echo.overlay.js
2 files changed, 30 insertions(+), 15 deletions(-)

Approvals:
  Bsitu: Looks good to me, but someone else must approve
  Kaldari: Looks good to me, approved
  EBernhardson (WMF): Looks good to me, but someone else must approve
  jenkins-bot: Verified



diff --git a/modules/overlay/ext.echo.overlay.css 
b/modules/overlay/ext.echo.overlay.css
index bd51cc4..83b32b8 100644
--- a/modules/overlay/ext.echo.overlay.css
+++ b/modules/overlay/ext.echo.overlay.css
@@ -29,26 +29,33 @@
        overflow: auto;
 }
 #p-personal .mw-echo-overlay li.mw-echo-notification {
-       background-color: #F1F1F1;
        display: block;
        float: none;
+       margin: 0;
+       padding: 0;
+}
+#p-personal .mw-echo-overlay li.mw-echo-notification 
.mw-echo-notification-wrapper {
+       display: block;
+       background-color: #F1F1F1;
        border-bottom: 1px solid #DDDDDD;
        padding: 15px 40px 10px 10px;
-       margin: 0;
        white-space: normal;
        font-size: 13px;
        line-height: 16px;
+       /* Suppress standard links styles */
+       color: inherit;
+       text-decoration: inherit;
 }
-#p-personal .mw-echo-overlay li.mw-echo-notification:hover {
+#p-personal .mw-echo-overlay li.mw-echo-notification:hover 
.mw-echo-notification-wrapper {
        background-color: #F9F9F9;
 }
-#p-personal .mw-echo-overlay li.mw-echo-notification.mw-echo-unread {
+#p-personal .mw-echo-overlay li.mw-echo-notification.mw-echo-unread 
.mw-echo-notification-wrapper {
        background-color: white;
 }
-#p-personal .mw-echo-overlay li.mw-echo-notification.mw-echo-unread:hover {
+#p-personal .mw-echo-overlay li.mw-echo-notification.mw-echo-unread:hover 
.mw-echo-notification-wrapper {
        background-color: #F9F9F9;
 }
-#p-personal .mw-echo-overlay li.mw-echo-notification:last-child {
+#p-personal .mw-echo-overlay li.mw-echo-notification:last-child 
.mw-echo-notification-wrapper {
        border-bottom: none;
 }
 #p-personal .mw-echo-title a {
diff --git a/modules/overlay/ext.echo.overlay.js 
b/modules/overlay/ext.echo.overlay.js
index f03f5ca..508a426 100644
--- a/modules/overlay/ext.echo.overlay.js
+++ b/modules/overlay/ext.echo.overlay.js
@@ -56,7 +56,8 @@
                                }
                                $ul.css( 'max-height', notificationLimit * 95 + 
'px' );
                                $.each( notifications.index, function ( index, 
id ) {
-                                       var data = notifications.list[id],
+                                       var $wrapper,
+                                               data = notifications.list[id],
                                                $li = $( '<li></li>' )
                                                        .data( 'details', data )
                                                        .data( 'id', id )
@@ -81,17 +82,24 @@
                                                }
                                        );
                                        // If there is a primary link, make the 
entire notification clickable.
+                                       // Yes, it is possible to nest <a> tags 
via DOM manipulation,
+                                       // and it works like one would expect.
                                        if ( $li.find( 
'.mw-echo-notification-primary-link' ).length ) {
-                                               $li.css( 'cursor', 'pointer' );
-                                               $li.click( function() {
-                                                       if ( 
mw.echo.clickThroughEnabled ) {
-                                                               // Log the 
clickthrough
-                                                               
mw.echo.logInteraction( 'notification-link-click', 'flyout', +data.id, 
data.type );
-                                                       }
-                                                       window.location.href = 
$li.find( '.mw-echo-notification-primary-link' ).attr( 'href' );
-                                               } );
+                                               $wrapper = $( '<a>' )
+                                                       .addClass( 
'mw-echo-notification-wrapper' )
+                                                       .attr( 'href', 
$li.find( '.mw-echo-notification-primary-link' ).attr( 'href' ) )
+                                                       .click( function() {
+                                                               if ( 
mw.echo.clickThroughEnabled ) {
+                                                                       // Log 
the clickthrough
+                                                                       
mw.echo.logInteraction( 'notification-link-click', 'flyout', +data.id, 
data.type );
+                                                               }
+                                                       } );
+                                       } else {
+                                               $wrapper = $('<div>').addClass( 
'mw-echo-notification-wrapper' );
                                        }
 
+                                       $li.wrapInner( $wrapper );
+
                                        mw.echo.setupNotificationLogging( $li, 
'flyout' );
 
                                        if ( !data.read ) {

-- 
To view, visit https://gerrit.wikimedia.org/r/77824
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I311eca70f025ce92129c828cd88f96686b7cff72
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/extensions/Echo
Gerrit-Branch: master
Gerrit-Owner: Matmarex <[email protected]>
Gerrit-Reviewer: Alex Monk <[email protected]>
Gerrit-Reviewer: Anomie <[email protected]>
Gerrit-Reviewer: Bsitu <[email protected]>
Gerrit-Reviewer: Cscott <[email protected]>
Gerrit-Reviewer: EBernhardson (WMF) <[email protected]>
Gerrit-Reviewer: Kaldari <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: Matmarex <[email protected]>
Gerrit-Reviewer: jenkins-bot

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to