Page MenuHomePhabricator

diff views do not wrap text, can be many times wider than browser width
Closed, ResolvedPublic

Description

Author: silsor

Description:
In diff view, lengthy lines of text (without whitespace) cause the view to expand
horizontally without restriction. This makes viewing the differences between
versions with long lines impractical.

Example:
http://en.wikipedia.org/w/index.php?title=Wikipedia&diff=next&oldid=9818898
(tested on KHTML)


Version: unspecified
Severity: normal
OS: other
Platform: Other
URL: http://en.wikipedia.org/w/index.php?title=Wikipedia&diff=next&oldid=9818898

Details

Reference
bz1438
ReferenceSource BranchDest BranchAuthorTitle
repos/abstract-wiki/wikifunctions/function-evaluator!65WASM-apartmainjforresterbuild: Switch main published images back to non-WASM, publish distinctly
repos/abstract-wiki/wikifunctions/function-evaluator!63T343829mainjforresterbuild: Switch published images to WASM-ified versions
repos/abstract-wiki/aw-e2e!8nik-55/download-logsmainnik-55Add artifact for e2e test logs
repos/phabricator/extensions!16T343923strlenPhp81wmf/stableaklapperMake some strlen() calls in OAuth code safe for PHP 8.0
repos/abstract-wiki/wikifunctions/function-evaluator!37no-js-evalmainjforresterDraft: javascript evaluator: Undefine 'eval' so users can't use it
Customize query in GitLab

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 8:12 PM
bzimport set Reference to bz1438.
bzimport added a subscriber: Unknown Object (MLST).
  • This bug has been marked as a duplicate of 1229 ***

smjg wrote:

Bug 1229 is about the widths of the diff columns relative to each other. This
is about the absolute widths of diff columns. Never mind that that bug has
received so many off-topic comments.

smjg wrote:

*** Bug 4494 has been marked as a duplicate of this bug. ***

Indeed, they may be wide if content is wide. That's why god invented scroll bars.
INVALID.

ui2t5v002 wrote:

(In reply to comment #4)

Indeed, they may be wide if content is wide. That's why god invented scroll bars.
INVALID.

Ummm... So the whole page has to be widened when only a single element is too
long? Scroll bars can be applied to individual elements, you know.

(not invalid)

smjg wrote:

Exactly. All too often I come across diffs that are hard to read because they
are too wide, generally because of very long URLs. This bug was filed in order
that a fix for this problem may be developed. See the off-topic discussion at
bug 1229 for more info.

Traditionally, the way around it was to use tinyurl. However, now tinyurl and
similar services are on the spam blacklist. I filed bug 4891 on how better to
deal with such URLs, but others so far don't seem to like my idea.

ayg wrote:

Hacky workaround (at least for Firefox): td.diff-context, td.diff-deletedline,
td.diff-addedline { overflow: auto; display: block; }. For other browsers, a
much slower and possibly buggy Javascript hack is at
http://en.wikipedia.org/wiki/Wikipedia:WikiProject_User_scripts/Scripts/Fix_diff_width.

blah wrote:

This is a valid bug (or at least usability problem). I think something should
be done about it. See:

http://en.wikipedia.org/wiki/Page_widening

While this doesn't fix the actual problem, in many cases the need for horizontal
scrolling could be significantly reduced by inserting soft word breaks in
certain locations, such as after slashes, in the diff text.

Unfortunately, the only markup for a soft word break that renders safely (i.e.
either as intended or not at all) in all browsers is the "<wbr>" tag, which is
not valid XHTML (see f.ex. http://www.quirksmode.org/oddsandends/wbr.html). I
believe brion at least has opposed its use for that reason. The second-best
alternative is probably "<span style="font-size:0px"> </span>", as used in
[[Template:Wbr]], but this renders as a visible space in non-CSS browsers. The
Unicode zero-width space (&#8203;) is even worse, since on most older and some
newer systems it renders as a very conspicuous "unknown character" box.

Did a little poking in r22192, which resolves this for Gecko and Opera, but not
for MSIE or KHTML. (See further comments on the commit.)

r22204 fixes this for MSIE, KHTML (Safari and Konqueror) and iCab. Yay!

I'm now using the fixed table layout style; this should also take care of bug 1229.

In r22227 I also added the 'word-wrap: break-word' CSS property, so we can get the desired wrapping behavior instead of the uglier scrolling in more cases.

This property is in CSS 3 draft, and supported by Internet Explorer (tested 6 and 7) and Safari.

Unfortunately Gecko/Firefox still doesn't support it:
https://bugzilla.mozilla.org/show_bug.cgi?id=99457

nor does Opera, but we can hope they'll add it eventually.

Regarding the rendering problems in IE: An explicit "width: 100%" might solve the lack of a scrollbar in IE 6. An explicit "overflow-y: visible" (with higher specificity than the general "overflow: auto") might solve the spurious vertical scrollbars in IE 6/7. These may, in turn, require the following IE-specific workaround to avoid the scrollbar overlapping the content:

table.diff td div { padding-bottom: expression(this.scrollWidth > this.offsetWidth ? "16px" : 0); }

(Note that all of this is guesswork, since I don't have IE to test with here. However, I did do some testing of these things back when I wrote [[Wikipedia:WikiProject_User_scripts/Scripts/Fix_diff_width]].)

There have been some reports of severe rendering problems involving the vertical collapse of table cells down to one line. One browser this has been reported on is Netscape 7.0 / Unix, which I'm currently installing to see if I can duplicate the issue.

Screenshot of broken diff rendering in Netscape 7.0

I've managed to reproduce the rendering bug on Netscape 7.0, see attached screenshot. The scrollbars are completely missing, and all table cells are reduced to a height of one line.

Attached:

Netscape7.0_broken_diff_rendering.png (563×747 px, 83 KB)

The rendering in Netscape 7.1 is correct; presumably this is a Gecko bug fixed between 20020823 and 20030624. I'm not sure there's much we can or should do, except to advise the affected users to upgrade.

There are no remaining IE 6.0 issues, since proper wrapping is now enforced there. The scrolling mode is only a workaround for browsers not supporting the forced wrapping.

The old Gecko problem is bug 9948, see my comments soon there.