History: Fix RTL layout in grouped history and some other minor fixes.

chrome/browser/resources/history/history.js

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 <include src="../uber/uber_utils.js">
 
 ///////////////////////////////////////////////////////////////////////////////
 // Globals:
 /** @const */ var RESULTS_PER_PAGE = 150;
 
@@ skipping 54 matching lines @@
   // Whether this is the continuation of a previous day.
   this.continued = continued;
 }
 
 // Visit, public: -------------------------------------------------------------
 
 /**
  * Returns a dom structure for a browse page result or a search page result.
  * @param {Object} propertyBag A bag of configuration properties, false by
  * default:
- * <ul>
- * <li>isSearchResult: Whether or not the result is a search result.</li>
- * <li>addTitleFavicon: Whether or not the favicon should be added.</li>
- * </ul>
+ *  - isSearchResult: Whether or not the result is a search result.
+ *  - addTitleFavicon: Whether or not the favicon should be added.
  * @return {Node} A DOM node to represent the history entry or search result.
  */
 Visit.prototype.getResultDOM = function(propertyBag) {
   var isSearchResult = propertyBag.isSearchResult || false;
   var addTitleFavicon = propertyBag.addTitleFavicon || false;
   var node = createElementWithClassName('li', 'entry');
   var time = createElementWithClassName('div', 'time');
   var entryBox = createElementWithClassName('label', 'entry-box');
   var domain = createElementWithClassName('div', 'domain');
 
@@ skipping 283 matching lines @@
  * Receiver for history query.
  * @param {Object} info An object containing information about the query.
  * @param {Array} results A list of results.
  */
 HistoryModel.prototype.addResults = function(info, results) {
   $('loading-spinner').hidden = true;
   this.inFlight_ = false;
   this.isQueryFinished_ = info.finished;
   this.queryCursor_ = info.cursor;
 
-  // If the results are not for the current search term there's nothing more
-  // to do.
+  // If the results are not for the current search term then there is nothing

[Patrick Dubroy, 2013/01/25 15:29:59]

Didn't want to do what James suggested? ;-)

[Sergiu, 2013/01/28 17:24:52]

I realized that I didn't like the sentence with or without the comma and found
it better with "then" in it :)

+  // more to do.
   if (info.term != this.searchText_)
     return;
 
   // If necessary, sort the results from newest to oldest.
   if (!results.sorted)
     results.sort(function(a, b) { return b.time - a.time; });
 
   var lastVisit = this.visits_.slice(-1)[0];
   var lastDay = lastVisit ? lastVisit.dateRelativeDay : null;
 
@@ skipping 297 matching lines @@
  * Record that the given visit has been rendered.
  * @param {Visit} visit The visit that was rendered.
  * @private
  */
 HistoryView.prototype.setVisitRendered_ = function(visit) {
   visit.isRendered = true;
   this.currentVisits_.push(visit);
 };
 
 /**
- * This function generates and adds the grouped visits DOM for a certain
- * domain. This includes the clickable arrow and domain name and the visit
- * entries for that domain.
+ * Generates and adds the grouped visits DOM for a certain domain. This
+ * includes the clickable arrow and domain name and the visit entries for
+ * that domain.
  * @param {Element} results DOM object to which to add the elements.
  * @param {string} domain Current domain name.
  * @param {Array} domainVisits Array of visits for this domain.
  * @private
  */
 HistoryView.prototype.getGroupedVisitsDOM_ = function(
     results, domain, domainVisits) {
   // Add a new domain entry.
   var siteResults = results.appendChild(
       createElementWithClassName('li', 'site-entry'));
   // Make a wrapper that will contain the arrow, the favicon and the domain.
   var siteDomainWrapper = siteResults.appendChild(
       createElementWithClassName('div', 'site-domain-wrapper'));
   var siteArrow = siteDomainWrapper.appendChild(
       createElementWithClassName('div', 'site-domain-arrow collapse'));
   siteArrow.textContent = '►';
   var siteDomain = siteDomainWrapper.appendChild(
       createElementWithClassName('div', 'site-domain'));
   var numberOfVisits = createElementWithClassName('span', 'number-visits');
   numberOfVisits.textContent = loadTimeData.getStringF('numbervisits',
                                                        domainVisits.length);
-  siteDomain.textContent = domain;
+  siteDomain.textContent = ' ' + domain + ' ';

[Patrick Dubroy, 2013/01/25 15:29:59]

Why are you adding space to both sides of the domain here? If it's to add space
before the parentheses and have it work in RTL, I think there are better ways.
Either use -webkit-margin-start  or -webkit-margin-end and set a margin of 1em,
or put   inside the span.

[Sergiu, 2013/01/28 17:24:52]

Put in a -webkit-margin-end but it may still not work (aka have the space)
properly on rtl languages. I'll test some more.

   siteDomain.appendChild(numberOfVisits);
   siteResults.appendChild(siteDomainWrapper);
   var resultsList = siteResults.appendChild(
       createElementWithClassName('ol', 'site-results'));
 
   domainVisits[0].addFaviconToElement_(siteDomain);
 
   siteDomainWrapper.addEventListener('click', toggleHandler);
   // Collapse until it gets toggled.
   resultsList.style.height = 0;
@@ skipping 22 matching lines @@
       visitsByDomain[domain] = [];
       domains.push(domain);
     }
     visitsByDomain[domain].push(visit);
   }
   var sortByVisits = function(a, b) {
     return visitsByDomain[b].length - visitsByDomain[a].length;
   };
   domains.sort(sortByVisits);
 
-  for (var i = 0, domain; domain = domains[i]; i++) {
+  for (var i = 0, domain; domain = domains[i]; i++)
     this.getGroupedVisitsDOM_(results, domain, visitsByDomain[domain]);
-  }
 };
 
 /**
  * Adds the results grouped by days, grouping them if needed.
  * @param {Array} visits Visits returned by the query.
  * @param {Element} parentElement Element to which to add the results to.
  * @private
  */
 HistoryView.prototype.addDayResults_ = function(visits, parentElement) {
   if (visits.length == 0)
@@ skipping 181 matching lines @@
  * Set the hash to a specified state, this will create an entry in the
  * session history so the back button cycles through hash states, which
  * are then picked up by our listener.
  * @param {string} term The current search string.
  * @param {number} page The page currently being viewed.
  * @param {boolean} grouped Whether the results are grouped or not.
  */
 PageState.prototype.setUIState = function(term, page, grouped) {
   // Make sure the form looks pretty.
   $('search-field').value = term;
-  if (grouped) {
+  if (grouped)
     $('display-filter-sites').checked = true;
-  } else {
+  else
     $('display-filter-sites').checked = false;
-  }
   var hash = this.getHashData();
   if (hash.q != term || hash.p != page || hash.g != grouped) {
     window.location.hash = PageState.getHashString(
         term, page, grouped);
   }
 };
 
 /**
  * Static method to get the hash string for a specified state
  * @param {string} term The current search string.
@@ skipping 319 matching lines @@
     historyView.reload();
 }
 
 // Add handlers to HTML elements.
 document.addEventListener('DOMContentLoaded', load);
 
 // This event lets us enable and disable menu items before the menu is shown.
 document.addEventListener('canExecute', function(e) {
   e.canExecute = true;
 });