485816 - codemirror syntax colorshift on Safari4b

Status: RESOLVED INVALID

(Skywriter Graveyard :: Syntax Highlighting, defect, P3)

Description

[Roberto Saccon]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_5_6; en-us) AppleWebKit/528.16 (KHTML, like Gecko) Version/4.0 Safari/528.16
Build Identifier: 195e9be4f6f3

codemirror syntaxengine on Safari4b does produce colors which are not identical to the defined ones. Compared to Firefox, were colors are correct, they look on Safari4b significantly less bright. 

This is a possible canvas / webworker async issue, where canvas context gets partially lost, and if so, there is no quick-fix for this, but the problem and possible solutions are currently investigated as part of thunderhead development by @danyat and myself.

Reproducible: Always

Comment 1

[Gordon P. Hemsley [:GPHemsley]]

I trust you know what you're talking about, so I'm confirming this.

Comment 2

[Julian Viereck]

I got the basic issue of this problem :)

I think it's much more easier than Roberto thought it would be. The issue is caused by the fact, that the last item in the lineInfo.regions array looks something like this:

{"plain":[{"start":undefined,"stop":undefined}

This makes the complete line text been painted with the style plain and "overpaints" the styles that were painted before. Because the text rendering in Safari is quite smooth, the syntax highlighting overpainted still gets through at some points and causes this blured colours.

To solve this, it would be the best the syntax engine wouldn't produce this last entry of the array ({"plain":[{"start":undefined,"stop":undefined}) as this is for no use. As I don't know to much about the syntax engine and I don't want to mix up Roberto's code, I wrote a little workaround, that's just one line of code.

Comment 3

[Julian Viereck]

Attachment: Fixes Codemirror issue with safari

patch as mentioned in comment #2

Comment 4

[Dion Almaer]

Pushed this change, we should still look into why the Code Mirror syntax check is sending that in.....

Comment 5

[Julian Viereck]

Attachment: Bugfix for workaround

The bugfix was doing a bad job when using the syntaxengine simple => only use the workaround when using CodeMirror.

Comment 6

[Dion Almaer]

Should we do this? Or isn't having range.start / range.stop both undefined a Bad Thing anyway, so we should just check for that regardless and not pass it through?

So, I was thinking of going the other way, from:

if (style == 'plain' && !range.start && !range.end) continue;

to:

if (!range.start || !range.end) continue;

(need to check to see if start/end can ever been undef).

Comment 7

[Julian Viereck]

Attachment: Better solution to make syntaxengine simple work ;)

Fixes the bug for syntaxengine codemirror as well the bug that came up with syntaxengine simple

Comment 8

[Dion Almaer]

Finalised on:

if (range.start === undefined && range.end === undefined) continue;

Comment 9

[Roberto Saccon]

I did some surgery and replaced the quickfix from comment 8 with a proper solution fixing the cause that created that problem.

I also fixed an internal bug in codemirror engine (for details see: http://groups.google.com/group/codemirror/browse_thread/thread/a76588c10a86b220)

and I moved cacheinvalidation form bespin.editor.action to bespin.editor.model (as discussed with Julian)

I pulled from Gordon the latest fixes (merged with tip, but not applied yet to main tip), added my changes (as MQ patch) and pushed (with MQ patch applied) to my repo:

http://bitbucket.org/rsaccon/bespin-playground/

Comment 10

[Dion Almaer]

Attachment: Patches from Gordon's latest

hg incoming is a scary list of changes that I don't trust. I did a fresh clone, a pull from Gordon, and did a diff to get this patch.

Does it look right for the changes?

Comment 11

[Roberto Saccon]

I cannot speak for how this patch affects the changes of Gordon and Julian, but this patch seems to take out all the changes I did (comment 9)

Comment 12

[Gordon P. Hemsley [:GPHemsley]]

(In reply to comment #10)
> Created an attachment (id=370363) [details]
> Patches from Gordon's latest
> 
> hg incoming is a scary list of changes that I don't trust. I did a fresh clone,
> a pull from Gordon, and did a diff to get this patch.
> 
> Does it look right for the changes?

No, it's reversed. You've got the changes going the wrong way.

Comment 13

[Julian Viereck]

(In reply to comment #10)

Dion,

> hg incoming is a scary list of changes that I don't trust.

this is the normal thing, when you pull from a repo that was merged agains the main repo. Just because there is a lot of things in hg incoming this *does not* mean there will be a lot changes when you do a hg merge. When performing hg merge after hg pull... you will find only the files merged, that were affected by Roberto, Gordon and me and not all the other files that are listed in incomming, since they are already at the same state than tip is (because they were merged once before against the main tip ;)).

Could you give it a try and pull from Roberto's repo, ignore hg incomming, and just do hg merge (might to a clone of tip at first). You will see, everything should be okay. That way all the changes Gordan, Roberto and I did are still there in their own changesets and you can figure out better, what changed where, by whom and such stuff. That's better than just applying one big patch ;)

Comment 14

[Gordon P. Hemsley [:GPHemsley]]

This is a mass migration from Mozilla Labs :: Bespin to Bespin :: Syntax Highlighting.

Comment 15

[Ben Galbraith]

is this issue resolved?

Comment 16

[Kevin Dangoor]

ACETRANSITION

The Skywriter project has merged with Ajax.org's Ace project (the full server part of which is their Cloud9 IDE project). Background on the change is here:

http://mozillalabs.com/skywriter/2011/01/18/mozilla-skywriter-has-been-merged-into-ace/

The bugs in the Skywriter product are not necessarily relevant for Ace and quite a bit of code has changed. For that reason, I'm closing all of these bugs. Problems that you have with Ace should be filed in the Ace issue tracker at GitHub:

https://github.com/ajaxorg/ace/issues