Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable word breaking for job names #725

Closed
wants to merge 2 commits into from

Conversation

olivergondza
Copy link
Member

Fix for JENKINS-17030.

This implementation inserts "break-marks" explicitly as I was not able to come up with purely CSS solution. AFAIK, word-wrap and break-word works only for elements having explicitly declared width.

Whether to use <wbr>, &shy; or some other alternative is a subject of discussion.

@cloudbees-pull-request-builder

core » jenkins_main_trunk #693 UNSTABLE
Looks like there's a problem with this pull request

@buildhive
Copy link

Jenkins » jenkins #28 ABORTED

(what's this?)

@cloudbees-pull-request-builder

core » jenkins_main_trunk #799 FAILURE
Looks like there's a problem with this pull request

/**
* Get a string that can be safely broken to several lines when necessary.
*
* This implementation inserts &ltwbr> tags into string. It allows browsers
Copy link
Member

Choose a reason for hiding this comment

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

Missing a ; here I think.

@jglick
Copy link
Member

jglick commented Mar 26, 2013

http://stackoverflow.com/questions/226464/soft-hyphen-in-html-wbr-vs-shy seems inconclusive. The wbr tag makes sense to me, but remember that Jelly output is X(HT)ML whereas you seem to be emitting HTML.

@olivergondza
Copy link
Member Author

Thank you for the review. You are right about the HTML. Using &#8203; entity instead of <wbr> seems valid and working in modern browsers and as a numbered entity, it should not cause problems with XML.

I will update the pull request accordingly.

@cloudbees-pull-request-builder

core » jenkins_main_trunk #803 UNSTABLE
Looks like there's a problem with this pull request

@jglick
Copy link
Member

jglick commented Apr 4, 2013

Rather than emit &#8203; why not emit the actual Unicode character (\u200b) and avoid any possible issues with metacharacters?

In which case there would be no reason for clients to escape anything; and breakable.jelly would be overkill: you could just write e.g.

<h1>${it.pronoun} ${h.breakableString(it.displayName)}</h1>

(This is all assuming that ZWSP is in fact a good replacement for <wbr>. SHY looks a bit better according to this page but seems less compatible.)

@olivergondza
Copy link
Member Author

It does look better, but can we assume we serve unicode? I will test it and create new PR, this one is cluttered.

@vjuranek
Copy link
Member

vjuranek commented Apr 5, 2013

@jglick
Copy link
Member

jglick commented Apr 5, 2013

Emitting the raw Unicode character should be fine.

By the way you should look into using soft breaking in core/src/main/resources/lib/hudson/propertyTable.jelly, e.g. <st:out value="${h.breakableString(e.value)}"/>. It is sorely needed for things like ${sun.boot.class.path} and $CATALINA_OPTS in /systemInfo. Demonstration:

"LONG_ENV=lots and lots of text here" mvn -f war/pom.xml -DskipTests=true "-Dlongsysprop=lots more text here" hudson-dev:run

@olivergondza
Copy link
Member Author

Here is what I found, after a bit of experimenting with Unicode characters:

\u200b - Breaks page search
\u00ad - Unicode equivalent of &shy;. It does what we want

http://jsfiddle.net/qY6mp/4/

@olivergondza
Copy link
Member Author

to @jglick : String containing \u00ad are copied with this characters (in firefox and chromium but not in Opera). This might be problematic for things like classpaths.

Sadly, I did not manage to find Unicode char that would allow line break, be ignored while searching and not appear in copied text. Not to mention we want this to work cross-browser.

@jglick
Copy link
Member

jglick commented Apr 26, 2013

In that case we are back to <wbr/> I guess. It seems to be supported in most browsers. The only downside I see is that it does not display a hyphen glyph indicating that the line was broken, but you could probably guess this anyway.

@olivergondza
Copy link
Member Author

I guess so. Unfortunately, this bring the ugly breakable.jelly back too.

@jglick
Copy link
Member

jglick commented May 1, 2013

Right, because ${…} will be escaped. Still, if it works then this does not seem too bad, assuming there is not a long list of places where it needs to be used.

@olivergondza
Copy link
Member Author

Replaced by #767

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants