Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 362286 - Lines in the ruler do not align with the lines of code
Summary: Lines in the ruler do not align with the lines of code
Status: RESOLVED FIXED
Alias: None
Product: Orion
Classification: ECD
Component: Editor (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 0.4 M1   Edit
Assignee: Mihai Sucan CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-28 05:53 EDT by Mihai Sucan CLA
Modified: 2011-12-01 17:00 EST (History)
3 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Mihai Sucan CLA 2011-10-28 05:53:21 EDT 
(cross post from https://bugzilla.mozilla.org/show_bug.cgi?id=679088 )

The lines in the ruler do not align with the lines of code.

Screenshot:
https://bugzilla.mozilla.org/attachment.cgi?id=553243

This happens with Monaco and Inconsolata on Macs. On Linux I can reproduce the issue with Monaco, but not with Inconsolata.
Comment 1 Mihai Sucan CLA 2011-10-28 06:36:45 EDT 
Proposed fix:

https://github.com/mihaisucan/orion.client/tree/bug-362286

Investigation yielded the following results:

- with Monaco 12pt, real line height was 21px.
  - calculateLineHeight gave 20px which caused the misalignment of the lines in the ruler and the lines of code.
  - that result also broke hit testing - one could click on the last visible line, but Orion would select two lines above, and other weird behavior. Anything using line height went wrong, of course.

- checked the situation and it seems that the spans generated by calculateLineHeight are indeed only 20px, it's only the line div that is taller.
  - checked if this was only weirdness during calculateLineHeight. it's not.
    - when the editor runs and the real line rendering happens I inspected the SPANs with Firebug. their computed height is indeed 20px. Again, only the line DIV has 21px.

Solution: since we can't rely on the spans height, I just grab the line DIV height in calculateLineHeight. That works now and it doesn't cause regressions with the other fonts.

Tested on Linux with Firefox, Opera and Chrome. Please let me know if any further changes are needed. This bug is now high priority for Mozilla (P1). Thank you!
Comment 2 Felipe Heidrich CLA 2011-11-04 10:50:55 EDT 
I am 99% sure Silenio and I had this code in the past.
The problem was that the height in the linediv was a bit too big (one pixel) in some cases. Leaving a visible white gab between selected lines. I suspect you can confirm this case by disabling full selection (see construction options) and selecting text in the editor (several lines). Try different fonts maybe.

Either way, I'm okay if you push your change to master as this bug is more serious than the original one (specially now that full selection hides the original bug anyway).
Comment 3 Mihai Sucan CLA 2011-11-04 11:59:56 EDT 
(In reply to comment #2)
> I am 99% sure Silenio and I had this code in the past.
> The problem was that the height in the linediv was a bit too big (one pixel) in
> some cases. Leaving a visible white gab between selected lines. I suspect you
> can confirm this case by disabling full selection (see construction options)
> and selecting text in the editor (several lines). Try different fonts maybe.

That can probably be fixed by setting line.style.lineHeight = computedHeight.


> Either way, I'm okay if you push your change to master as this bug is more
> serious than the original one (specially now that full selection hides the
> original bug anyway).

Thank you! I will push the fix.