Page MenuHomePhabricator
Create Task
Maniphest T12163

Long lines overflow in Diffusion browser
Closed, ResolvedPublic
Actions

Description

Possibly related to T11236?

Reproduction steps:

  • Make a commit with some arbitrarily-long (>225 characters or so) line of text without spaces.
  • Browse that file in Diffusion at /diffusion/REPO/browse/BRANCH/MYFILE.TXT

Expected result:

  • Long line wraps at some reasonable width

Observed result:

  • Long line does not wrap and the side column is hidden underneath the Diffusion source table:

pasted_file (428×1 px, 42 KB)

Our version information:

  • rP23c54262caf84abdad82974212170ff24d138f4d
  • rARC9503b941cc02be637d967bb50cfb25f852e071e4
  • rPHU10963f771f118baa338aacd3172aaede695cde62

Version information on Phacility test instance:

  • phabricator 23c54262caf84abdad82974212170ff24d138f4d (Sun, Jan 22) (branched from ddf82a815b9a07e901870c2f4d5b7582af7b4d82 on origin)
  • arcanist 9503b941cc02be637d967bb50cfb25f852e071e4 (Fri, Jan 6) (branched from ade25facfdf22aed1c1e20fed3e58e60c0be3c2b on origin)
  • phutil 10963f771f118baa338aacd3172aaede695cde62 (Fri, Jan 13) (branched from 9d85dfab0f532d50c2343719e92d574a4827341b on origin)

Revisions and Commits

Restricted Differential RevisionRestricted Diffusion Commit

Related Objects

Event Timeline

faulconbridge created this task.Jan 27 2017, 3:32 PM
epriestley added a subscriber: epriestley.Jan 27 2017, 4:47 PM

In Paste (and I think Differential), we scroll lines instead of trying to wrap them:

Screen Shot 2017-01-27 at 7.51.30 AM.png (255×1 px, 40 KB)

In theory, it might be nice to unify the Diffusion file view code and the paste file view code. However, the Diffusion stuff at least is pretty complex/specialized so I'm not sure this is desirable in practice.

A sufficient near-term fix is probably sticking overflow-x: auto; into CSS somewhere.

chad added a project: Differential.Jan 27 2017, 5:27 PM
Herald added a subscriber: eadler. · View Herald TranscriptJan 27 2017, 5:27 PM
epriestley edited projects, added Diffusion; removed Differential.Jan 27 2017, 5:29 PM

(I think this only impacts Diffusion, not Differential.)

chad added a subscriber: chad.Jan 27 2017, 7:01 PM

typeaheads are useless

epriestley added a comment.Jan 27 2017, 7:02 PM

sorry that our DEEP NEURAL NETWORK TECHNOLOGY can not yet READ MINDS

chad added a comment.Jan 27 2017, 7:03 PM

I'm just going to rename Differential everywhere.

eadler added a comment.Jan 27 2017, 7:03 PM
In T12163#208938, @epriestley wrote:

sorry that our DEEP NEURAL NETWORK TECHNOLOGY can not yet READ MINDS

https://archive.google.com/mentalplex/

chad added a project: PHUI.Feb 2 2017, 2:07 AM
epriestley moved this task from Backlog to v3 on the Diffusion board.Feb 2 2017, 3:20 PM
epriestley edited projects, added Diffusion (v3); removed Diffusion.
epriestley moved this task from Backlog to Probably Actual Bugs on the Diffusion (v3) board.Feb 2 2017, 3:53 PM
chad added a comment.Feb 2 2017, 10:00 PM

I can't reproduce this, is there a specific browser/OS I should test? Also, feel free to upload a test file to rGITTEST

chad added a comment.EditedFeb 2 2017, 10:01 PM

word-break: break-all; seems to be working fine.

pasted_file (1×2 px, 388 KB)

chad edited projects, added Bug Report (Needs Information); removed Bug Report.Feb 2 2017, 10:03 PM
epriestley added a comment.Feb 2 2017, 10:05 PM

Safari / OSX:

https://secure.phabricator.com/diffusion/GITTEST/browse/master/wide.txt

Screen Shot 2017-02-02 at 2.04.57 PM.png (1×1 px, 140 KB)

chad added a comment.Feb 2 2017, 10:05 PM

I see, it must be a text file.

chad added a revision: Restricted Differential Revision.Feb 2 2017, 10:09 PM
chad edited projects, added Bug Report; removed Bug Report (Needs Information).Feb 2 2017, 10:10 PM
chad closed this task as Resolved by committing Restricted Diffusion Commit.Feb 2 2017, 10:11 PM
chad added a commit: Restricted Diffusion Commit.
Phabricator · Built by Phacility