Closed
Bug 779642
Opened 12 years ago
Closed 12 years ago
Scratchpad Line numbers don't line up
Categories
(DevTools Graveyard :: Scratchpad, defect, P2)
DevTools Graveyard
Scratchpad
Tracking
(firefox17 verified)
RESOLVED
FIXED
Firefox 18
Tracking | Status | |
---|---|---|
firefox17 | --- | verified |
People
(Reporter: rcampbell, Assigned: capella)
References
Details
(Keywords: regression)
Attachments
(4 files)
(deleted),
image/png
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
rcampbell
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Regression introduced with the landing of bug 756423. On Mac, the line numbers no longer line up due to the additional height of the command key on Macs.
Reporter | ||
Comment 1•12 years ago
|
||
Reporter | ||
Comment 2•12 years ago
|
||
One possible solution for this would be to change the string returned for the Command Key to "Command" or "Cmd".
Comment 3•12 years ago
|
||
Ouch. This happens because Orion, at initialization, checks the line height and it assumes all chars have the same height. Unfortunately the Command Key character is of a different height, breaking a lot of things in Orion (not just line numbers alignment).
Good suggestion - we should just say "Command" or "Cmd".
Comment 4•12 years ago
|
||
So if the user types or pastes this (or a similar) character, Orion will lose it again? Would using the height of this character (or a similar "big" one) for initialization work?
Comment 5•12 years ago
|
||
I noticed some fonts have different "big" characters. we can't know which is the biggest char in each font. The actual solution is to fix how Orion works internally. Afaik, they will allow variable line heights in the future.
Additionally, if you don't use any of the big chars in your lines but you determine the line height using a big char during init.... you still have the same problem.
Reporter | ||
Comment 6•12 years ago
|
||
looks like mihai is going to file upstream. We should do the keystring fix mentioned above in the meantime.
Filter on BLACKEAGLE?!
Priority: -- → P3
Reporter | ||
Updated•12 years ago
|
Priority: P3 → P2
Comment 7•12 years ago
|
||
Reported upstream:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=386517
Comment 8•12 years ago
|
||
Changing "⌘" to "Cmd" wouldn't solve the problem, since the user can type this character.
On the other hand, changing the font we use to Menlo, which is the monospaced font that
Apple wants everybody to have as a default anyway, would fix it for good.
Besides, it is a great font.
Comment 9•12 years ago
|
||
For some reason, using this fix makes my font look big.
Does anyone have the same issue?
Reporter | ||
Comment 10•12 years ago
|
||
Comment on attachment 649048 [details] [diff] [review]
Using the Menlo font fixes the issue.
unfortunately, we can't do that. We should respect the system's font-settings. I would personally love to use Menlo, but if a user picks a different font...
Reporter | ||
Comment 11•12 years ago
|
||
and, fwiw, my own system has Menlo specified here.
Assignee | ||
Comment 12•12 years ago
|
||
I just noticed this.
Until orion ever opts to go for variable line height, I'd like to change this to use "Cmd" to at least fix the regression caused by my patch.
This still follows Mac convention, and if the user inserts a cloverleaf char into the text, the problem is of his own doing, and is an exposure that currently exists on the orion side / not of our doing.
Reporter | ||
Comment 13•12 years ago
|
||
(In reply to Mark Capella [:capella] from comment #12)
> I just noticed this.
>
> Until orion ever opts to go for variable line height, I'd like to change
> this to use "Cmd" to at least fix the regression caused by my patch.
>
> This still follows Mac convention, and if the user inserts a cloverleaf char
> into the text, the problem is of his own doing, and is an exposure that
> currently exists on the orion side / not of our doing.
yeah, I'd like that until we get a proper fix for line-heights. I think that's the way to go. Feel like taking it?
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → markcapella
Status: NEW → ASSIGNED
Assignee | ||
Comment 14•12 years ago
|
||
Temp Fix - Override Mac |cloverleaf| char with |Cmd-| string, until Orion fixes variable line height.
Attachment #654863 -
Flags: review?(rcampbell)
Reporter | ||
Comment 15•12 years ago
|
||
Comment on attachment 654863 [details] [diff] [review]
Patch (v1)
+ // Use "Cmd-" literal vs cloverleaf meta-key until
+ // Orion adds variable height lines
+ //elemString += keysbundle.GetStringFromName("VK_META_CMD") +
+ // keysbundle.GetStringFromName("MODIFIER_SEPARATOR");
could you reference this bug in the comment? You can preface with a XXX bug 779642 on your first comment line. Also add a space between the comment // and elemString.
r+ with those nits fixed.
Attachment #654863 -
Flags: review?(rcampbell) → review+
Assignee | ||
Comment 16•12 years ago
|
||
As pushed to TRY...
https://tbpl.mozilla.org/?tree=Try&rev=3b2d1b5509ad
Assignee | ||
Comment 17•12 years ago
|
||
And on to inbound:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=98a07ac71ddc
Comment 18•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 18
Comment 19•12 years ago
|
||
We need to fix this in Aurora as well, right?
status-firefox17:
--- → affected
Reporter | ||
Comment 20•12 years ago
|
||
Comment on attachment 655794 [details] [diff] [review]
Patch (v2)
[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 756423
User impact if declined: users's line numbers won't line up in the scratchpad.
Testing completed (on m-c, etc.): on m-c, try
Risk to taking this patch (and alternatives if risky): users won't like the scratchpad.
String or UUID changes made by this patch: none (internal, non l10n strings only)
Attachment #655794 -
Flags: approval-mozilla-aurora?
Comment 21•12 years ago
|
||
Comment on attachment 655794 [details] [diff] [review]
Patch (v2)
[Triage Comment]
New feature, early in the cycle, low risk fix. Approved for Aurora 17.
Attachment #655794 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 22•12 years ago
|
||
Comment 23•12 years ago
|
||
Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/17.0 Firefox/17.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:17.0) Gecko/17.0 Firefox/17.0
Verified with Ubuntu, Mac OS 10.7 and Windows 7. No problems with rendering and lining up characters with line numbers if the apple key character isn't used.
Updated•6 years ago
|
Product: Firefox → DevTools
Updated•5 years ago
|
Product: DevTools → DevTools Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•