YUILibrary - Open source JavaScript and CSS for building richly interactive software.
Fork YUI on GitHub

YUI 2.x

Ticket #2527720 (closed defect)

Reporter


K o v a c s 		Opened: 02/24/09
Last modified: 09/14/09
Status: closed
Type: defect
Resolution: fixed

Owner


Satyam 		Target Release: 2.8.0
Priority: P3 (normal)
Summary: TreeView 2.7.0 mishandles href
Description:

I had a TreeView 2.6.0 built from HTML, e.g.:

{type:'html', html:'<a href="./a.html" title="A" target="c">A</a>'},

where target="c" was an iframe in a center panel on the same page. This worked as expected: when the user left-clicked a TreeView node that was a href, the documented was opened in the center layout.

A left-click on a node with a href no longer works as expected in TreeView 2.7.0 -- the only way to actually view the hyperlinked document is to use a right-click to open it in a new tab or new
window.

Note: I only tested type:'html' -- T.B.D. if it's an issue with a text node

2.7.0 failed in FF 2.0.20, IE 6, and Chrome (all were WinXP) while
2.6.0 worked in all those browsers.

Other ref.:

http://tech.groups.yahoo.com/group/ydn-javascript/message/45563
Type: defect Observed in Version: 2.7.0
Component: TreeView Severity: S3 (normal)
Assigned To: Satyam Target Release: 2.8.0
Location: Priority: P3 (normal)
Tags: Relates To:
Browsers: All
URL:
Test Information:

Attachments

Attachment #1: layout0A_for_YUILibrary.htm (download)

Change History

LJones
Posted: 02/26/09

I can confirm with IE 7.0.6001.18000, Chrome 1.0.154.48, Safari, Firefox 3.0.6, IE8rc2 on winVista x32

IE8rc on win7 (beta build 7000) x64

file type makes no difference, it appears to be due to a change in the click event code for the label.

http://developer.yahoo.com/yui/examples/treeview/tv-markup.html

Satyam

YUI Contributor
Posted: 03/3/09
  • status changed from new to accepted

Satyam

YUI Contributor
Posted: 03/4/09
  • owner changed from Satyam to Adam Moore
  • status changed from accepted to assigned

Patch submitted on GitHub, waiting for merge or further fixing.

My fault, by checking so much the new features, I completely overlooked the basic features, which I took for granted. Sorry for the mess.

Eric Miraglia

YUI Developer
Posted: 03/5/09

GitHub reference:

http://github.com/Satyam/yui2/commit/e070afb6eee31af40df6df2f2f1612a1d1573cf6

Adam Moore

YUI Developer
Posted: 03/5/09
  • milestone changed to 2.NEXT
  • status changed from assigned to checkedin

K o v a c s
Posted: 03/13/09

I re-pulled from Github at this URL: http://tinyurl.com/cnc9n3 -- this is the "Fri Feb 27 10:26:35 -0800 2009 removed &nbps; [apm]" version. It built the treeview but the hyperlinked nodes still don't work--the only way to get the hyperlinked document to open is to right-click on the hyperlink and choose open in new tab/window. (Same situation as my original post.)

Adam Moore

YUI Developer
Posted: 03/13/09
  • status changed from checkedin to reopened

Adam Moore

YUI Developer
Posted: 03/13/09
  • owner changed from Adam Moore to Satyam
  • status changed from reopened to assigned

Adam Moore

YUI Developer
Posted: 03/13/09
  • status changed from assigned to checkedin

K o v a c s
Posted: 03/14/09

I just pulled the latest and NO JOY--same issue as my original post.

I did a diff with the version I pulled earlier in the day. And just for the heck of it, I changed this code:

_onClickEvent: function (ev) {
var self = this,
td = this._getEventTargetTdEl(ev),
node,
target,
toggle = function (force) {
node.focus();
if (force || !node.href) {

to this:

if (force || node.href) {

(i.e., got rid of the `!') and guess what? IT WORKED! Of course I have no idea what I may have inadvertently broken by my hack. Anyway left-clicking the href node now opens the linked document in the center panel--just like TreeView 2.6.0 used to do.

K o v a c s
Posted: 03/14/09

P.S. Side-effect: GitHub silently removed the boolean OR operators from the code snippet in my previous reply. Sigh. I.e.: if (force OR node.href) {

Adam Moore

YUI Developer
Posted: 03/14/09
  • status changed from checkedin to reopened

Reopening. Can you provide the file you are working with so we can verify your particular problem is truly fixed?

K o v a c s
Posted: 03/15/09

Replying to [comment:12 adam]:
> Reopening. Can you provide the file you are working with so we can verify your particular problem is truly fixed?

My six (+/-) attempts to "Attach file" resulted in a core dump of YUILibrary. Sigh. I'll try another time. { Well, that didn't help. And just submitting this reply is taking multiple attempts. } Here's the code:

<html>
<head>

<!-- Individual YUI CSS files -->
<link rel="stylesheet" type="text/css" href="http://yui.yahooapis.com/2.7.0/build/reset-fonts-grids/reset-fonts-grids.css">
<link rel="stylesheet" type="text/css" href="http://yui.yahooapis.com/2.7.0/build/assets/skins/sam/skin.css">
<!-- Individual YUI JS files -->
<script type="text/javascript" src="http://yui.yahooapis.com/2.7.0/build/yahoo-dom-event/yahoo-dom-event.js"></script>
<script type="text/javascript" src="http://yui.yahooapis.com/2.7.0/build/dragdrop/dragdrop-min.js"></script>
<script type="text/javascript" src="http://yui.yahooapis.com/2.7.0/build/element/element-min.js"></script>
<script type="text/javascript" src="http://yui.yahooapis.com/2.7.0/build/resize/resize-min.js"></script>
<script type="text/javascript" src="http://yui.yahooapis.com/2.7.0/build/layout/layout-min.js"></script>

<script type="text/javascript" src="http://yui.yahooapis.com/2.7.0/build/treeview/treeview-min.js"></script>

<!--Local copy with patch applied-->
<!--<script type="text/javascript" src="treeview.js"></script>-->

<style>
.yui-skin-sam .yui-layout .yui-layout-unit div.yui-layout-bd {
background-color:white;
}

</style>

</head>

<body class="yui-skin-sam">

<div id="l"></div>
<div id="center"><iframe id="cc" name="c" width="100%" height="100%"></iframe></div>

<script>
(function() {
var Dom = YAHOO.util.Dom,
Event = YAHOO.util.Event;

Event.onDOMReady(function() {
var layout = new YAHOO.widget.Layout({
units: [
{
position: 'left',
header: 'Links',
width: 200,
gutter: '5px',
collapse: true,
scroll: true,
body: 'l'
},
{
position: 'center',
body: 'center'
}
]
});
layout.render();
});
})();

var tree = new YAHOO.widget.TreeView("l",[
//NOTE: Clicking the PDF icon in this example code causes a 404 NOT FOUND;
//which of course does NOT happen in my actual implementation.
//I included the hyperlinked PDF icon in case it is part of the issue.
{type:'html', html:'<a href="http://yuilibrary.com/projects/yui2/ticket/2527720" target="c">Active Ticket</a> <a href="a.pdf" target="c"><img src="http://www.adobe.com/images/pdficon_small.gif" height=17 width=17></a>'},
{type:'html', html:'<a href="http://tech.groups.yahoo.com/group/ydn-javascript/message/46413" target="c">ydn-javascript</a> <a href="b.pdf" target="c"><img src="http://www.adobe.com/images/pdficon_small.gif" height=17 width=17></a>'},
{type:'html', html:'<a href="http://satyam.com.ar/yui/" target="c">Examples</a> <a href="c.pdf" target="c"><img src="http://www.adobe.com/images/pdficon_small.gif" height=17 width=17></a>'},
{type:'text', label:'Yahoo!', children:[{type:'html', html:'<a href="http://www.yahoo.com/" target="c">Yahoo!</a> <a href="d.pdf" target="c"><img src="http://www.adobe.com/images/pdficon_small.gif" height=17 width=17></a>'},{type:'html', html:'<a href="http://yq.search.yahoo.com/" target="c">Search</a> <a href="e.pdf" target="c"><img src="http://www.adobe.com/images/pdficon_small.gif" height=17 width=17></a>'},{type:'html', html:'<a href="http://developer.yahoo.com/" target="c">Dev Network</a> <a href="f.pdf" target="c"><img src="http://www.adobe.com/images/pdficon_small.gif" height=17 width=17></a>'}]},

]);

tree.render();

</script>

</body>
</html>

Satyam

YUI Contributor
Posted: 03/15/09
  • status changed from reopened to checkedin

The report that caused the reopening was because the /build folder at GitHub had failed to get synched with the /src, thus, the patch was not there.

Once that was fixed, why downloading the source from /build stripped an OR operator from the source has no explanation I can figure and is no fault of TreeView itself. I downloaded it myself with no errors.

The source in the message right above has two problems:

a) It loads the released version of TreeView which is known to be in error. This might be due to a misunderstanding which might be worth clarifying. A new build available on GitHub does not affect the released files at yahooapis.com. Intermediate builds don't change the released files. If you need a patch from a build, you have to pull it from GitHub and serve it from your own network.

b) The tree nodes used are of type HTML. TreeView doesn't know there are links in them you want to respond to. In fact, TreeView knows nothing about such nodes and does the default action. To suppress this behavior, you just need to add the following line:

tree.subscribe('clickEvent',function () {return false;});

This is a different issue from the one initially reported, which really was a bug. TreeView wasn't navigating on TextNodes with the href property set, that is an error. It is not an error that it does the default action when not suppressed.

K o v a c s
Posted: 03/15/09

Replying to [comment:14 satyam]:
> The report that caused the reopening was because the /build folder at GitHub had failed to get synched with the /src, thus, the patch was not there.
>

Understood.

> Once that was fixed, why downloading the source from /build stripped an OR operator from the source has no explanation I can figure and is no fault of TreeView itself. I downloaded it myself with no errors.
>

satyam:

The issue was: the logical-OR was removed by the software that is used to handle the "Submit changes" button on this page. I.e., after I posted my code modification on this ticket (i.e., my "reply"), I noticed the logical-OR operator had been stripped.

http://yuilibrary.com/projects/yui2/ticket/2527720#comment:11

In other words: the issue with the logical-OR operator had nothing to do with downloading code from Github.

> The source in the message right above has two problems:
>
> a) It loads the released version of TreeView which is known to be in error. This might be due to a misunderstanding which might be worth clarifying. A new build available on GitHub does not affect the released files at yahooapis.com. Intermediate builds don't change the released files. If you need a patch from a build, you have to pull it from GitHub and serve it from your own network.
>

Adam asked me for the code I was using so I posted it--presumably because someone was going to run it and experiment with it. In order to facilitate that activity I opted to include not only the (knowingly) defective

(a) http://yui.yahooapis.com/2.7.0/build/treeview/treeview-min.js

but also this (b):

<!--Local copy with patch applied-->
<!--<script type="text/javascript" src="treeview.js"></script>-->

I thought by doing so that someone would (perhaps) use (a) to establish the baseline that yeah, it's busted--but _you_ already knew that--still maybe someone else may be interested in observing this behavior. (Witness the two separate threads on ydn-javascript as an example.)

As for your comment, "If you need a patch from a build, you have to pull it from GitHub and serve it from your own network." I would have hoped that my inclusion of (b) above would have convinced you that yes I am aware of that necessity.

> b) The tree nodes used are of type HTML. TreeView doesn't know there are links in them you want to respond to. In fact, TreeView knows nothing about such nodes and does the default action. To suppress this behavior, you just need to add the following line:
>
> tree.subscribe('clickEvent',function () {return false;});
>
> This is a different issue from the one initially reported, which really was a bug. TreeView wasn't navigating on TextNodes with the href property set, that is an error. It is not an error that it does the default action when not suppressed.

The code I posted has both text *and* HTML nodes. (Note also that my original post was about HTML nodes.) Anyway the as-is code I posted worked perfectly with 2.6.0. (without the tree.subscribe bit you mentioned above).

After Adam announced the /build and /src issue had been worked out, I re-pulled the code and still my code did not work like it did using 2.6.0:

http://yuilibrary.com/projects/yui2/ticket/2527720#comment:10

I made one modification (i.e., deleting `!') to the code I downloaded (and locally served) and it started working like it did using 2.6.0.

Anyway:

Did you actually run and test my code with your patch?

Did you try it with 2.6.0 to verify that it does in fact correctly work without the `tree.subscribe' bit?

K o v a c s
Posted: 03/18/09

The courtesy of a reply is requested for this question:

Why does my code work as it is, unmodified, with 2.6.0 but not with 2.7.0?

K o v a c s
Posted: 03/18/09

Replying to [comment:16 kovacs]:
> The courtesy of a reply is requested for this question:
>
> Why does my code work as it is, unmodified, with 2.6.0 but not with 2.7.0?

Clarification: I was referring to Satyam's patched version of TreeView 2.7.0

(Which I re-downloaded from /build and re-tested with my code; still NO JOY.)

Adam Moore

YUI Developer
Posted: 03/18/09

kovacs, I reopened the bug per your report. This ticket will be updated when your answer is available.

Satyam

YUI Contributor
Posted: 03/19/09

In searching for another bug I realized that this thread was still going on. I am subscribed via email and I should have received a notification but didn't. Might this be related to the scripts failing? Anyway, I'll report it elsewhere as well.

I'm not really understanding the issue about the missing OR. If there is a problem with uploading or downloading code, a bug report on TreeView is not the place to deal with it.

The behavior on 2.7.0 is the correct one. The behavior on 2.6.0 was in error and it was fixed in response to ticket #2144989:

http://yuilibrary.com/projects/yui2/ticket/2144989

Kovacs code does work with 2.6 without listening to clickEvent, but that was because of this particular bug and it should not be taken as a guide on how 2.7 should behave.

Canceling the default behavior by listening to clickEvent as indicated is the way to go.

If any other behavior is desired, a new ticket for a feature request can be filed.

Mat Ludlam
Posted: 07/3/09

I have read the above and I am not sure if this is resolved or not. If not then I have some additional information that may be of interest:

The example at: http://developer.yahoo.com/yui/examples/treeview/tv-edit_clean.html

Includes a node that links to the YAHOO home page. This works just fine with a single left click in FF 3.5 and IE8. In the example it creates some event handlers for "double click" and some other events, and adding these makes the href to work. If you take the event handlers out then it stops working.

I have a much simpler example of a failure at: http://ludlams.dyndns.org/YuiTreeView.html

George

YUI Developer
Posted: 07/10/09
  • milestone changed from 2.NEXT to 2.8.0

George

YUI Developer
Posted: 09/14/09
  • status changed from checkedin to closed

2.8.0 has been released. All "checkedin" items are available for download in the official release. Status of "checkedin" items is being set to closed.

George

YUI Developer
Posted: 09/14/09
  • resolution changed to fixed