Page MenuHomePhabricator

Use `<td class="n" data-n="3">` instead of `<th>3</th>` for line numbers
ClosedPublic

Authored by epriestley on Feb 16 2019, 7:37 PM.
Tags
None
Referenced Files
F14415463: D20188.id48196.diff
Tue, Dec 24, 6:01 PM
Unknown Object (File)
Tue, Dec 17, 7:40 AM
Unknown Object (File)
Fri, Dec 13, 9:27 PM
Unknown Object (File)
Sun, Dec 8, 6:54 PM
Unknown Object (File)
Fri, Dec 6, 11:16 AM
Unknown Object (File)
Wed, Dec 4, 2:19 AM
Unknown Object (File)
Sun, Dec 1, 2:10 PM
Unknown Object (File)
Tue, Nov 26, 7:37 PM
Subscribers
None

Details

Summary

Ref T13161. Ref T12822. See PHI870. Long ago, the web was simple. You could leave your doors unlocked, you knew all your neighbors, crime hadn't been invented yet, and <th>3</th> was a perfectly fine way to render a line number cell containing the number "3".

But times have changed!

  • In PHI870, this isn't good for screenreaders. We can't do much about this, so switch to <td>.
  • In D19349 / T13105 and elsewhere, this ::after { content: attr(data-n); } approach seems like the least bad general-purpose approach for preventing line numbers from being copied. Although Differential needs even more magic beyond this in the two-up view, this is likely good enough for the one-up view, and is consistent with other views (paste, harbormaster logs, general source display) where this technique is sufficient on its own.

The chance this breaks something is pretty much 100%, but we've got a week to figure out what it breaks. I couldn't find any issues immediately.

Test Plan
  • Created, edited, deleted inlines in 1-up and 2-up views.
  • Replied, keyboard-navigated, keyboard-replied, drag-selected, poked and prodded everything.

Diff Detail

Repository
rP Phabricator
Branch
sorcery1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 22044
Build 30114: Run Core Tests
Build 30113: arc lint + arc unit