This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 130512 - open html tag get red background
Summary: open html tag get red background
Status: CLOSED FIXED
Alias: None
Product: web
Classification: Unclassified
Component: HTML Editor (show other bugs)
Version: 6.x
Hardware: Sun All
: P1 blocker (vote)
Assignee: Marek Fukala
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-03-19 12:53 UTC by Jindrich Sedek
Modified: 2009-05-18 10:47 UTC (History)
3 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
screenshot (125.22 KB, image/png)
2008-04-01 17:50 UTC, Jindrich Sedek
Details
At attribute's value. (3.17 KB, image/gif)
2008-04-04 16:36 UTC, Martin Schovanek
Details
At end tag. (2.84 KB, image/gif)
2008-04-04 16:37 UTC, Martin Schovanek
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jindrich Sedek 2008-03-19 12:53:06 UTC
one some HTML file
type "<p "
whole area from p to next HTML tag gets red background
Comment 1 Jindrich Sedek 2008-03-26 09:44:36 UTC
increasing priority, it's very visible and bothering issue during editing 
Comment 2 Marek Fukala 2008-03-26 15:38:27 UTC
fixed in changeset 08fef45ead2d
Comment 3 Jindrich Sedek 2008-04-01 17:50:12 UTC
tags where end tag is not required are still having red background. 
use this code:
---------
  <head>
        <|meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
  </head>
-----------
set cursor possition at possition of "|"
whole meta tag get's red beckground

I'm not sure if such behavior was expected, but the red color indicates some error a the code is correct because end 
tag is forbidden in such situation.
red color also makes the tag hard to read - see screenshot

I would propose at least changing the color
Comment 4 Jindrich Sedek 2008-04-01 17:50:52 UTC
Created attachment 59499 [details]
screenshot
Comment 5 Marek Fukala 2008-04-01 18:11:43 UTC
Looks like a problem in HtmlElementHandle.signatureEquals(). 
Comment 6 Marek Fukala 2008-04-01 18:15:00 UTC
please disregard my last comment, it should come to a different issue
Comment 7 Marek Fukala 2008-04-01 19:13:41 UTC
You are right, we are not checking the DTD in the braces matching at all. This is wrong, DTD needs to be taken into
account to filter such situations. Is this a P1 for 6.1? I would say so.
Comment 8 Jindrich Sedek 2008-04-01 19:19:14 UTC
I agree fixing this in 6.1. 
Comment 9 Jindrich Sedek 2008-04-02 14:37:02 UTC
One more situation is marking tag background red is adding some attribute. It's also not very pleasant, because it's
hard to read newly written text
Comment 10 Marek Fukala 2008-04-02 19:07:28 UTC
This issue is quite tricky. Basically the current implementation of braces matching is divided into two methods -
findOrigin() and findMatches(). If findOrigin returns null no pair is searched then. So I can easily check if a selected
tag has optional end or start and return null in findOrigin() but then, if the tag HAS pair tag it won't be selected. If
the findOrigin() retuns non-null value (mean something matchable is selected) then the findMatches() can either return
null (no pair found - the selected origin will be red backgrounded) or non-null value - the pair. From this description
is clear that I cannot implement the desired behaviour using the API in the way how it was ment to be used. 

The usecases:
1) a tag is selected and it HAS to have a pair according to DTD - this scenario is OK
2) a tag is selected and it CAN have a pair according to DTD - in findOrigin() I have to return the selected tag
boundaries since it can have the pair. However if I do that and there is no pair - what to return in findMatches()????
null will mark the origin as unmatched and I do not have any pair.

There should be IMHO a way how the matcher can say "ok, there was an origin, I cannot find a pair, but it is OK". In
such case the origin will not be highlighted. Something like return new int[]{Integer.MAX_VALUE, Integer.MAX_VALUE}
would work thought its ugly. 

I have partially workarounded the case #2 by returning new int[]{context.getSearchOffset(), context.getSearchOffset()}
in findMatches() if there is no pair and there doesn't have to be one. It looks like the infrastructure survives that
and the only sideeffect is that the origin is selected, which in some sense may make sense. Jumping to pair action
doesn't move the caret, which is OK.

I will push the workaround to main and ask QE to verify, but I would like to hear Vita's opinion first. Maybe there is
more elegant way how to do that.
Comment 11 Marek Fukala 2008-04-02 19:08:44 UTC
BTW I could also fix the case #2 by doing the search of the pair in findOrigin() method, but I guess this is not how it
should work at all ;-)
Comment 12 Marek Fukala 2008-04-02 19:20:22 UTC
Another problem is what Jindra mentioned in his last comment - if you put the caret into a tag with a pair and start
typing in the tag content (for example adding attributes) the tag gets red. This is because of there is no actual parser
result after each keystroke when findOrigin is called. Once the parser result is ready, I have no way how to say to the
infrastructure that there is a pair now. This is IMO another argument for using push model instead of the pull one for
the braces matching api. The first reason is the fact that the document is readlocked when findMatches() is called which
prevents me to access the GSF parser safely.

I will workaround this issue in the same way as for the usecase #2 - the text will be yellow during typing, BUT even if
there isn't any end tag and there has to be :-(. 
Comment 13 Marek Fukala 2008-04-02 19:26:27 UTC
the described workarounds pushed to main in revision 1b7a51fde936.
Comment 14 Martin Schovanek 2008-04-02 21:07:31 UTC
The main reason why this issue is P1 is that the attributes' values are almost *UNREADEBLE* (see the screeshot) on the
red background. Vito/Marku can we change the color to be lighter or use a different color eq Cyan or Green?
Comment 15 Jindrich Sedek 2008-04-03 10:11:13 UTC
I'm trying the workaround in main. It's much better. I see one more problem that was originally mentioned in #131871:

HTML tags are not matched if there is a javascript used inside and both tags are becoming red and unreadable:
--------
        <select onclick="arguments()"> 
        </select>
--------

I would consider this issue as fixed for 6.1 and previous sample should be fixed in #131871.
Comment 16 Vitezslav Stejskal 2008-04-03 18:33:34 UTC
Re. the locking problem - please see issue #131284.

Re. usecase #2 - I agree that this is not supported well. The workaround you did is ok. I think we should allow
returning an empty array from findMatcher(), which would mean - 'there is no matching area and it's ok'. It is different
from what you suggest - "ok, there was an origin, I cannot find a pair, but it is OK" - which in fact is a lie, because
there *is* an origin, which may or may not have a matching area. And I think it should be highlighted. Alternatively, if
you were highlighting only '<tag' from <tag attrib="value" /> you could mark the ending '/>' as the matching area.

Which brings me to the comment about the colors and unreadable attributes. I think that matching the whole tag is not a
good idea at all, because the tag could span several lines and the highlight would be very very ugly. Probably even with
much lighter color. We intentionally chose very distinct color to make it visible when the origin is only one letter
wide, ie {([.

I don't want this to sound like I don't want to help you guys, but you have to understand that whatever we do we have to
make sure that it does not break things for others.
Comment 17 Marek Fukala 2008-04-03 18:48:16 UTC
>I think we should allow returning an empty array from findMatcher(), which would mean - 'there is no matching area and 
>it's ok'. 
But if I understand how it works the findMatches() are called only if findOrigin() returned non-null value which is IMO
exactly what I mean saying "ok, there was an origin, I cannot find a pair, but it is OK" - which 
>in fact is a lie
No it isn't. It is correct - There was an origin so I returned it. Then the infrastructure calls findMatches() which
realizes that the origin has no match, than it checks if the match is mandatory and if so colors the origin in red
otherwise does nothing.

In another words 'there is no matching area and it's ok' ==  "ok, there was an origin, I cannot find a pair, but it is OK"

I probably made a mistake, the sentece should be "ok, there is an origin, ....". I meant it this was, sorry for the bad
english.

As for the highlighting of just the tag name - as far as I remembed I tried this but the infrastructure complained if I
returned an origin not containing the search offset.

An example:

<body bgcolor="red">

</body>

If I put the caret into the bgcolor I still want the </body> to be highlighted. I'll doublecheck if it works...
Comment 18 Marek Fukala 2008-04-03 18:57:52 UTC
WARNING [org.netbeans.modules.editor.bracesmatching.MasterMatcher]: Origin offsets out of range, origin = [294, 299],
caretOffset = 310, lookahead = 2, searching forward. Offending BracesMatcher:
org.netbeans.modules.html.editor.HTMLBracesMatching@ad862b

...if I return origin out of the searchOffset.

The usecase I described in my previous record is valid. I am not sure if it would be good to match the tag only if you
are in the tagname itself. That would be a regression at least. Jindro and Martine, what do you think?
Comment 19 Jindrich Sedek 2008-04-03 19:16:58 UTC
there was an issue #121146 complaining about the behavior of matching just tag names. I'm afraid it really isn't correct
to match just name because of navigation, but I agree that few lines html tag looks ugly with red or other colored
colored background see issue #65554... Would it be possible to color just tag name, but navigate using Ctrl + '[' for
whole tag area?

Otherwise I agree with matching just tag name if it helps this issue and we should ask some HIE to define the best
behavior for this situation.
Comment 20 Marek Fukala 2008-04-03 19:28:26 UTC
I also agree with highlighting the tagname if the navigation to end tag works, but as I already wrote this is another
thing the infrastructure doesn't allow. It worked this way in 6.0.
Comment 21 Marek Fukala 2008-04-03 23:12:34 UTC
marking as fixed (in main)
Comment 22 Martin Schovanek 2008-04-04 16:34:56 UTC
Marku can you look at .xml implementation, it highlights only the start tag name. Se the attached screenshots.
Comment 23 Martin Schovanek 2008-04-04 16:36:58 UTC
Created attachment 59695 [details]
At attribute's value.
Comment 24 Martin Schovanek 2008-04-04 16:37:22 UTC
Created attachment 59696 [details]
At end tag.
Comment 25 Marek Fukala 2008-04-06 18:30:12 UTC
>Marku can you look at .xml implementation, it highlights only the start tag name. Se the attached screenshots.
Martine, the XML matching still uses the old infrastructure (SyntaxSupport.findMatchingBlock) which allowed this. You
can simply spot recognize this by the fact the origin is not highlighted, just the match. 

As I said the 6.0 implementation worked the same way, but after the new api adoption it cannot anymore. I belive Vita
will redesign or at least fix the API so we can have something reasonable in 6.(x>1).

I consider this a approved by QE, commiting to 6.1
Comment 26 Marek Fukala 2008-04-06 19:30:52 UTC
transplanted to release61 revision 692136f7dd35
Comment 27 Jindrich Sedek 2008-04-07 11:29:44 UTC
verified.
NetBeans IDE Dev (Build 200804062002)