Skip to content

Commit

Permalink
Fixed needing double def of term when data-lt is used (closes #470)
Browse files Browse the repository at this point in the history
  • Loading branch information
marcoscaceres committed Jul 31, 2015
1 parent 8ba09d9 commit f374d3f
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 23 deletions.
33 changes: 18 additions & 15 deletions js/core/utils.js
Expand Up @@ -44,16 +44,18 @@ define(
$.fn.getDfnTitles = function ( args ) {
var titles = [];
var theAttr = "";
var titleString = "";
if (this.attr("data-lt")) {
var titleString = "";
var normalizedTextContent = utils.normalizeString(this.text());
// allow @lt to be consistent with bikeshed
if (this.attr("data-lt") || this.attr("lt")) {
theAttr = this.attr("data-lt") ? "data-lt" : "lt";
// prefer @data-lt for the list of title aliases
titleString = this.attr("data-lt");
theAttr = "data-lt";
}
else if (this.attr("lt")) {
// allow @lt to be consistent with bikeshed
titleString = this.attr("lt");
theAttr = "lt";
titleString = this.attr(theAttr);
// Use the definition itself as first item, so to avoid
// having to declare the definition twice.
if(!this.attr(theAttr).startsWith(normalizedTextContent)) {
titleString = normalizedTextContent + "|" + titleString;
}
}
else if (this.attr("title")) {
// allow @title for backward compatibility
Expand All @@ -70,11 +72,7 @@ define(
titleString = this.text();
}
// now we have a string of one or more titles
titleString = titleString.toLowerCase() // mask to lower case
.replace(/^\s+/, "") // strip out any leading whitespace
.replace(/\s+$/, "") // and any trailing whitespace
.split(/\s+/) // split on any whitepace
.join(" "); // and replace with a single space
titleString = utils.normalizeString(titleString);
if (args && args.isDefinition === true) {
// if it came from an attribute, replace that with data-lt as per contract with Shepherd
if (theAttr) {
Expand Down Expand Up @@ -199,7 +197,12 @@ define(
}
return ret;
}

, normalizeString: function(str) {
return str.toLowerCase() // mask to lower case
.trim() // trim trailing/leading whitespace
.split(/\s+/) // split on any whitespace
.join(" "); // and replace with a single space
}
// Takes a string, applies some XML escapes, and returns the escaped string.
// Note that overall using either Handlebars' escaped output or jQuery is much
// preferred to operating on strings directly.
Expand Down
35 changes: 27 additions & 8 deletions tests/spec/core/utils-spec.js
Expand Up @@ -134,14 +134,31 @@ describe("Core - Utils", function () {
});
});

// $.getDfnTitles()
it("should find the data-lts", function () {
runs(function () {
var $dfn = $("<dfn data-lt='DFN|DFN2|DFN3'><abbr title='ABBR'>TEXT</abbr></dfn>").appendTo($("body"));
var titles = $dfn.getDfnTitles( { isDefinition: true } );
expect(titles[0]).toEqual("text");
expect(titles[1]).toEqual("dfn");
expect(titles[2]).toEqual("dfn2");
expect(titles[3]).toEqual("dfn3");
$dfn.removeAttr("data-lt");
expect($dfn.getDfnTitles()[0]).toEqual("abbr");
$dfn.find("abbr").removeAttr("title");
expect($dfn.getDfnTitles()[0]).toEqual("text");
$dfn.remove();
});
});
// $.getDfnTitles()
it("should find the definition title", function () {
runs(function () {
var $dfn = $("<dfn lt='DFN|DFN2|DFN3'><abbr title='ABBR'>TEXT</abbr></dfn>").appendTo($("body"));
var titles = $dfn.getDfnTitles( { isDefinition: true } );
expect(titles[0]).toEqual("dfn");
expect(titles[1]).toEqual("dfn2");
expect(titles[2]).toEqual("dfn3");
expect(titles[0]).toEqual("text");
expect(titles[1]).toEqual("dfn");
expect(titles[2]).toEqual("dfn2");
expect(titles[3]).toEqual("dfn3");
$dfn.removeAttr("data-lt");
expect($dfn.getDfnTitles()[0]).toEqual("abbr");
$dfn.find("abbr").removeAttr("title");
Expand All @@ -155,11 +172,13 @@ describe("Core - Utils", function () {
runs(function () {
var $dfn = $("<dfn lt='DFN|DFN2|DFN3'>TEXT</dfn>").appendTo($("body"));
var titles = $dfn.getDfnTitles( { isDefinition: true } );
expect(titles[0]).toEqual("dfn");
expect(titles[1]).toEqual("dfn2");
expect(titles[2]).toEqual("dfn3");
expect($dfn.attr("data-lt")).toEqual('dfn|dfn2|dfn3');
expect($dfn.getDfnTitles()[0]).toEqual("dfn");
expect(titles[0]).toEqual("text");
expect(titles[1]).toEqual("dfn");
expect(titles[2]).toEqual("dfn2");
expect(titles[3]).toEqual("dfn3");
console.log(titles, $dfn.attr("data-lt"), $dfn.getDfnTitles())
expect($dfn.attr("data-lt")).toEqual("text|dfn|dfn2|dfn3");
expect($dfn.getDfnTitles()[1]).toEqual("dfn");
$dfn.remove();
});
});
Expand Down

1 comment on commit f374d3f

@halindrome
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Please sign in to comment.