Page MenuHomePhabricator

Differential highlighting highlights wrong lines when setup warning is ignored by lackadaisical admins
Closed, ResolvedPublic

Description

When selecting lines inside of a diff, it seems that the yellow box to indicate which rows are highlighted is off-by-one.

This is in Chrome 42.0.2311.82 beta (64-bit) on OS X 10.10.3.

Event Timeline

mattrobenolt raised the priority of this task from to Needs Triage.
mattrobenolt updated the task description. (Show Details)
mattrobenolt added a subscriber: mattrobenolt.
chad added a subscriber: chad.Apr 15 2015, 6:21 PM

What browser? Can you reproduce the issue here?

mattrobenolt updated the task description. (Show Details)Apr 15 2015, 6:23 PM

@chad, updated browser/OS in description, and no, I can't reproduce here. We last updated to master yesterday, then again this morning hoping it was resolved.

chad added a comment.Apr 15 2015, 6:25 PM

Generally, we don't support non stable browsers. Does it reproduce with normal Chrome?

Yeah, I've tested in every browser I have on my computer. Chrome, Firefox, and Safari. All the same issue.

avivey added a subscriber: avivey.Apr 15 2015, 6:38 PM

Do you have any special settings for "monospaced font" https://secure.phabricator.com/settings/panel/display/ ?

chad added a comment.Apr 15 2015, 6:54 PM

All diffs on your local install, or just specific diffs?

It seems to be all diffs. But I can confirm later today if I find any that don't cause a problem.

Are you an administrator, with an "Unresolved Setup Issues..." banner showing on every page?

mattrobenolt added a comment.EditedApr 15 2015, 6:58 PM

@epriestley YUP, is that the problem? :)

I need to implement T6526 so I can award myself a MASTER SLEUTH badge.

(I'm not sure if it's actually the problem, but it seems likely.)

Yeah, confirmed. If I just delete the entire .setup-warning-callout div, it's fine.

chad added a comment.Apr 15 2015, 7:01 PM

We need T7567 so we can make the setup warning include an airraid.wav file.

(This token is meant to go on epriestley's comment, but we can't do that yet either...)

chad renamed this task from Differential highlighting highlights wrong lines to Differential highlighting highlights wrong lines when setup warning is ignored by lackadaisical admins.Apr 15 2015, 7:42 PM
chad added projects: Differential, Setup.

Just a small and unnecessary rhetorical question here: do people really leave setup warnings & ignore them? I mean - i fix every setup warning the moment they appear and assumed same is true for everybody. I really wish that @chad's wish for airraid.wav would come true ;)

chad added a comment.Apr 15 2015, 8:10 PM

I think we've chased our tails twice this week alone on the issue.

In fairness to the admins, we pushed a "deprecated Conduit methods called in the last 30 days" warning recently which often isn't easy to fix since you may have to chase people down or update things.

I can't remember exactly what the issue was, but there's something funky with work boards and setup issues as well.

chad added a comment.Apr 15 2015, 11:35 PM

Issues with Workboards and Conpherence were resolved earlier this week.

In T7830#107243, @chad wrote:

Issues with Workboards and Conpherence were resolved earlier this week.

Ah cool, I don't think I've updated recently.

In fairness to the admins, we pushed a "deprecated Conduit methods called in the last 30 days" warning recently which often isn't easy to fix since you may have to chase people down or update things.

This is exactly why we have the banner on our install. :) Normally, we don't have outstanding issues.

This has definitely fixed the issue, but introduced a new, much less annoying issue. :)

It seems to be that only on the Differential view, the little sidebar divider is now hovering over the banner.

Seems if I bump the z-index to 5, it displays correctly over the divider.