1087314 - htmlmin step of grunt build removes visible whitespace in the UI

Status: VERIFIED FIXED

(Tree Management :: Treeherder, defect, P2)

Description

[:Gijs (he/him)]

"Oct 22, 2014 8:55:08 AM [e-mail]" is weird. Either there should be a space after the dash, or the dash should go away. I'd lean towards the former.

Comment 1

[Jonathan French (:jfrench)]

Thanks Gijs! It is indeed like that on stage and production. I see it appears to be correct on dev, and in master in
http://treeherder-dev.allizom.org
https://github.com/mozilla/treeherder-ui/blob/2ebb18e215f28345dc1224f54e7ff8d68df1afb2/webapp/app/partials/main/jobs.html#L19

I think dev just needs to be pushed to stage and production. What is a mystery is the last time that line was touched I think was 27 days ago, and we have pushed to prod since then. I've added :camd in case there was/is some grunt issue.

I will check it the next push.

Comment 2

[Jonathan French (:jfrench)]

We pushed to production yesterday, but the change still isn't present on stage/prod, vs. dev - which is correct. Mauro, do you think there may be something getting missed during a grunt build here? Or something else perhaps?

Comment 3

[Ed Morley [:emorley]]

Nothing has landed recently to "fix" this, so I would instead imagine that the grunt build (which does minification and other processing), is incorrectly trimming the whitespace from the markup.

Comment 4

[Ed Morley [:emorley]]

Compare the grunt build version (running on stage and prod):
https://github.com/mozilla/treeherder-ui/blob/master/dist/js/index.min-599dc30eceb1ec84010748e11377e1a2.js

<span class=timestamp-name><span><a href={{revisionResultsetFilterUrl}} title=\"open this resultset in new tab\" target=_blank ng-bind=resultsetDateStr></a> -</span><th-author author={{resultset.author}}></th-author></span></span>

With the original:
https://github.com/mozilla/treeherder-ui/blob/2ebb18e215f28345dc1224f54e7ff8d68df1afb2/webapp/app/partials/main/jobs.html#L19

 <span class="timestamp-name">
<span>
<a href="{{revisionResultsetFilterUrl}}"
title="open this resultset in new tab"
target="_blank"
ng-bind="resultsetDateStr"></a> - </span>
<th-author author="{{resultset.author}}"></th-author>
</span>
 </span>

We probably need to tweak the htmlmin settings here:
https://github.com/mozilla/treeherder-ui/blob/2ebb18e215f28345dc1224f54e7ff8d68df1afb2/Gruntfile.js#L131

Or else use a &nbsp;

Comment 5

[Jonathan French (:jfrench)]

Thanks for investigating that Ed! Awesome.

Comment 6

[Jonathan French (:jfrench)]

It looks like the flag 'conservative-collapse' may do what we want.
https://www.npmjs.org/package/html-minifier

It preserves the trailing character in that span, and make it look correct in both local/dev(original) and stage/prod(minified).

It introduces new single character white spaces between some of the markup tags, but diffing the before and after minified index files it seems benign. I will attach those for reference.

Other options I tried;   and also alternate css padding. They made the minified look correct, but the un-minified local/dev(original) look too wide.

I'll put up a PR also.

Comment 7

[Jonathan French (:jfrench)]

Attachment: index.min-current.js

Comment 8

[Jonathan French (:jfrench)]

Attachment: index.min-conservativeCollapse.js

Comment 9

[Jonathan French (:jfrench)]

Attachment: treeherder-ui-PR#250

Please see the above PR and output minified results (attached above) for review and status.

Comment 10

[Treeherder GitHub Bugbot]

Commits pushed to master at https://github.com/mozilla/treeherder-ui

https://github.com/mozilla/treeherder-ui/commit/9ae24f3aff73f4de33b7a015d1a52359384311cc
Bug 1087314 - Preserve visible UI whitespace in grunt builds

https://github.com/mozilla/treeherder-ui/commit/13e5417fa2a802b9953afa0af945da49c88a1ff5
Merge pull request #250 from tojonmz/timestamp-whitespace

Bug 1087314 - Preserve visible UI whitespace in grunt builds

Comment 11

[Jonathan French (:jfrench)]

Interestingly, the fix shows up on prod, but not on stage (at least as of this evening).

Comment 12

[Ed Morley [:emorley]]

Stage hasn't been updated yet.

(venv)[treeherder@stage-1 treeherder-ui]$ git show -s
commit 19a695f29de0c6eaee81b86d6323148c86a0dc8c
Merge: e12443f 847dda8
Author: camd <cdawson@~snip~.com>
Date:   Mon Oct 27 10:53:00 2014 -0700

    Merge pull request #245 from mozilla/bug-1076769-parsing-logs-on-demand

    Bug 1076769 - support parsing a log on demand

Comment 13

[Jonathan French (:jfrench)]

Verified fixed and working correctly on stage and prod.

Comment 14

[Treeherder GitHub Bugbot]

Commits pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/427589a0f8fa1b7a53887f0110e9032dcdd1c2e3
Bug 1087314 - Preserve visible UI whitespace in grunt builds

https://github.com/mozilla/treeherder/commit/dbd4225cd3122d3c7cb1a480e47961dac5fd6afc
Merge pull request #250 from tojonmz/timestamp-whitespace

Bug 1087314 - Preserve visible UI whitespace in grunt builds