Closed
Bug 1087314
Opened 10 years ago
Closed 10 years ago
htmlmin step of grunt build removes visible whitespace in the UI
Categories
(Tree Management :: Treeherder, defect, P2)
Tree Management
Treeherder
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: Gijs, Assigned: jfrench)
References
()
Details
Attachments
(3 files)
"Oct 22, 2014 8:55:08 AM -cbook@mozilla.com" is weird. Either there should be a space after the dash, or the dash should go away. I'd lean towards the former.
Assignee | ||
Comment 1•10 years ago
|
||
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.
Assignee | ||
Updated•10 years ago
|
Priority: -- → P5
Assignee | ||
Comment 2•10 years ago
|
||
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?
Flags: needinfo?(mdoglio)
Comment 3•10 years ago
|
||
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•10 years ago
|
||
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
Updated•10 years ago
|
Summary: Add space or remove dash in treeherder header containing timestamp + committer → htmlmin step of grunt build removes visible whitespace in the UI
Updated•10 years ago
|
Assignee | ||
Comment 5•10 years ago
|
||
Thanks for investigating that Ed! Awesome.
Assignee | ||
Comment 6•10 years ago
|
||
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.
Assignee | ||
Comment 7•10 years ago
|
||
Assignee | ||
Comment 8•10 years ago
|
||
Assignee | ||
Comment 9•10 years ago
|
||
Please see the above PR and output minified results (attached above) for review and status.
Attachment #8511547 -
Flags: review?(cdawson)
Comment 10•10 years ago
|
||
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
Updated•10 years ago
|
Attachment #8511547 -
Flags: review?(cdawson) → review+
Updated•10 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 11•10 years ago
|
||
Interestingly, the fix shows up on prod, but not on stage (at least as of this evening).
Comment 12•10 years ago
|
||
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
Assignee | ||
Comment 13•10 years ago
|
||
Verified fixed and working correctly on stage and prod.
Status: RESOLVED → VERIFIED
Comment 14•9 years ago
|
||
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
You need to log in
before you can comment on or make changes to this bug.
Description
•