1152414 - encode job filter search url's whitespaces separatoring parameters to support pasting it in documents etc.

Status: RESOLVED FIXED

(Tree Management :: Treeherder, defect)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

The link at the bottom left 'Job: <job classes>' with tooltip 'Filter jobs like these' contains whitespaces. To better support pasting in documents etc. while keeping the actual filter, the whitespaces should be encoded.

Comment 1

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: pull request, v1

Comment 2

[Jonathan French (:jfrench)]

This sounds like our already open bug 1134240.

Comment 3

[Ed Morley [:emorley]]

Comment on attachment 8589749 [details] [review]
pull request, v1

I don't know if we want to do this? The root cause is bug 1148861 - with that, the links should be encoded as we copy them. Or does the html spec say we should encode the links ourselves?

Comment 4

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Bug 1148861 is about copying from the location bar - copying from the job panel at the bottom left would remain unchanged.

> Or does the html spec say we should encode the links ourselves?
Whitespaces are commonly used as delimiters of urls and so have to be encoded to not split the url (outside the location bar). See http://tools.ietf.org/html/rfc3986

Comment 5

[Ed Morley [:emorley]]

This WFM in Nightly now that the backout has occurred in bug 1149913. Though it still doesn't in Chrome, so I guess we should take it regardless.

Comment 6

[Ed Morley [:emorley]]

Comment on attachment 8589749 [details] [review]
pull request, v1

Cameron: This looks fine to me, but could you take a quick look to make sure I'm not missing something else that is affected by the change - since you're more familiar with this code than I am :-)

Comment 7

[Jonathan French (:jfrench)]

I had a look at current behaviors out of interest, on OSX 10.10.3 on:

Release 37.0.2 (which afaict should not have the fix for bug 1149913)
Nightly 40.0a1 (which afaict should have the fix)
Chrome Release (which for me has always worked fine and encoded url whitespace)
..running Release/Nightly concurrently with separate profiles

with:

Treeherder Production https://treeherder.mozilla.org
Archaeopteryx's PR branch running w/local server

test job/filter:
"Linux mozilla-central build", others

Expected behaviors, working correctly (All configurations and branches):
1) Job: "(name)" 'hover-copy' works, preserves white space as it should for pasting into IRC, emails, per Sheriff request when hovering over the string and issuing ctrl/cmd+c.

2) Clicking on Job: "(name)" to filter, or entering a filter by hand in Filter UI, presents the result with white space in the url awesome-bar, but when copied and pasted it is correctly %encoded% on Release and Nightly.

2a) Chrome additionally presents the url results in its bar as %encoded%, which is its normal behavior.

Could it be that bug 1149913 didn't get a tracking flag for a fix in 37? Given everything seems to work without the PR (ie. production is working with Release/Nightly) I am not clear if we should be encoding it in Treeherder, but I defer to you all. :)

Apologies if this adds to the confusion, but those were my test results.

Comment 8

[Ed Morley [:emorley]]

Use right-click copy link, rather than copying from the URL or using hover-copy :-)

Comment 9

[Jonathan French (:jfrench)]

Cool, will try.

Comment 10

[Jonathan French (:jfrench)]

Yup, so I agree with Ed's comment 5. Chrome's RMB->Copy Link Address, on Job: '(name)', omits the encoding during paste on current production. With the PR it appears to encode it correctly.

So Firefox Release, Nightly, is fine without this PR; Chrome could benefit from it.

Comment 11

[Treeherder GitHub Bugbot]

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

https://github.com/mozilla/treeherder-ui/commit/032e42eb0258c7f2e287c0e8a826a5c554fb2228
Bug 1152414 -  encode job filter search url's whitespaces separatoring parameters to support pasting it in documents etc.

Comment 12

[Cameron Dawson [:camd]]

Comment on attachment 8589749 [details] [review]
pull request, v1

I tested this out and it seems to work quite well.  Thanks!!

Comment 13

[Treeherder GitHub Bugbot]

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

https://github.com/mozilla/treeherder/commit/1962abca558179fb65a69a4b5c6bacfd4818091b
Bug 1152414 -  encode job filter search url's whitespaces separatoring parameters to support pasting it in documents etc.