[10] Review Request: 8182410, 8183508, 8181289

classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

[10] Review Request: 8182410, 8183508, 8181289

Sergey Bylokhov

Hello,
Please review the fix for.
8182410: missing 'title' in
api/javax/swing/plaf/synth/doc-files/componentProperties.html
8183508: multi_tsc.html should be updated
8181289: Invalid HTML 5 in AWT/Swing docs

Description:
  - Illegal characters were removed.
  - Unsupported tags/properties were removed -like <tt>, <center>, font,
etc.(except the tags related to tables which I'll fix later).
  - HTML5 doctype is set for all files.
  - The <title> is set for all files.
  - <a name="" is replaced by <a id=""
  - Copyrights were added to some files.

Note that I placed a <head> tag before copyright to solve errors like:
"A charset attribute on a meta element found after the first 1024 bytes.
Fatal Error: Changing encoding at this point would need non-streamable
behavior"

specdiff:
http://cr.openjdk.java.net/~serb/8181289/specdiff/overview-summary.html

Bugs:
     https://bugs.openjdk.java.net/browse/JDK-8182410
     https://bugs.openjdk.java.net/browse/JDK-8183508
     https://bugs.openjdk.java.net/browse/JDK-8181289

Webrev can be found at: http://cr.openjdk.java.net/~serb/8181289/webrev.00

--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review Request: 8182410, 8183508, 8181289

Semyon Sadetsky
Hi Sergey,

I see no reason to have an extra empty anchor tag to set a bookmark. The
id attribute works with any element.

For example:

     <a id="Definitions"></a>
     <h3>Definitions</h3>

should be

     <h3 id="Definitions">Definitions</h3>

--Semyon

On 10/23/2017 02:42 PM, Sergey Bylokhov wrote:

>
> Hello,
> Please review the fix for.
> 8182410: missing 'title' in
> api/javax/swing/plaf/synth/doc-files/componentProperties.html
> 8183508: multi_tsc.html should be updated
> 8181289: Invalid HTML 5 in AWT/Swing docs
>
> Description:
>  - Illegal characters were removed.
>  - Unsupported tags/properties were removed -like <tt>, <center>,
> font, etc.(except the tags related to tables which I'll fix later).
>  - HTML5 doctype is set for all files.
>  - The <title> is set for all files.
>  - <a name="" is replaced by <a id=""
Why you replace

>  - Copyrights were added to some files.
>
> Note that I placed a <head> tag before copyright to solve errors like:
> "A charset attribute on a meta element found after the first 1024
> bytes. Fatal Error: Changing encoding at this point would need
> non-streamable behavior"
>
> specdiff:
> http://cr.openjdk.java.net/~serb/8181289/specdiff/overview-summary.html
>
> Bugs:
>     https://bugs.openjdk.java.net/browse/JDK-8182410
>     https://bugs.openjdk.java.net/browse/JDK-8183508
>     https://bugs.openjdk.java.net/browse/JDK-8181289
>
> Webrev can be found at:
> http://cr.openjdk.java.net/~serb/8181289/webrev.00
>

Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review Request: 8182410, 8183508, 8181289

Sergey Bylokhov
Hi, Semyon
I guess I did not add a new <a> tags, it seems that this tag is used as
an anchor everywhere and since it is allowed in html5 its usage was
preserved.

On 23/10/2017 22:08, Semyon Sadetsky wrote:

> Hi Sergey,
>
> I see no reason to have an extra empty anchor tag to set a bookmark. The
> id attribute works with any element.
>
> For example:
>
>      <a id="Definitions"></a>
>      <h3>Definitions</h3>
>
> should be
>
>      <h3 id="Definitions">Definitions</h3>
>
> --Semyon
>
> On 10/23/2017 02:42 PM, Sergey Bylokhov wrote:
>>
>> Hello,
>> Please review the fix for.
>> 8182410: missing 'title' in
>> api/javax/swing/plaf/synth/doc-files/componentProperties.html
>> 8183508: multi_tsc.html should be updated
>> 8181289: Invalid HTML 5 in AWT/Swing docs
>>
>> Description:
>>  - Illegal characters were removed.
>>  - Unsupported tags/properties were removed -like <tt>, <center>,
>> font, etc.(except the tags related to tables which I'll fix later).
>>  - HTML5 doctype is set for all files.
>>  - The <title> is set for all files.
>>  - <a name="" is replaced by <a id=""
> Why you replace
>
>>  - Copyrights were added to some files.
>>
>> Note that I placed a <head> tag before copyright to solve errors like:
>> "A charset attribute on a meta element found after the first 1024
>> bytes. Fatal Error: Changing encoding at this point would need
>> non-streamable behavior"
>>
>> specdiff:
>> http://cr.openjdk.java.net/~serb/8181289/specdiff/overview-summary.html
>>
>> Bugs:
>>     https://bugs.openjdk.java.net/browse/JDK-8182410
>>     https://bugs.openjdk.java.net/browse/JDK-8183508
>>     https://bugs.openjdk.java.net/browse/JDK-8181289
>>
>> Webrev can be found at:
>> http://cr.openjdk.java.net/~serb/8181289/webrev.00
>>
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review Request: 8182410, 8183508, 8181289

Semyon Sadetsky
On 10/24/2017 12:02 PM, Sergey Bylokhov wrote:

> Hi, Semyon
> I guess I did not add a new <a> tags, it seems that this tag is used
> as an anchor everywhere and since it is allowed in html5 its usage was
> preserved.
Starting version 9 JDK supports the html5 markup in javadoc, so the
files need to be updated according to it. In html5 you need <a> only if
you want to see hyperlink and your fix is modifying those tags.

--Semyon

>
> On 23/10/2017 22:08, Semyon Sadetsky wrote:
>> Hi Sergey,
>>
>> I see no reason to have an extra empty anchor tag to set a bookmark.
>> The id attribute works with any element.
>>
>> For example:
>>
>>      <a id="Definitions"></a>
>>      <h3>Definitions</h3>
>>
>> should be
>>
>>      <h3 id="Definitions">Definitions</h3>
>>
>> --Semyon
>>
>> On 10/23/2017 02:42 PM, Sergey Bylokhov wrote:
>>>
>>> Hello,
>>> Please review the fix for.
>>> 8182410: missing 'title' in
>>> api/javax/swing/plaf/synth/doc-files/componentProperties.html
>>> 8183508: multi_tsc.html should be updated
>>> 8181289: Invalid HTML 5 in AWT/Swing docs
>>>
>>> Description:
>>>  - Illegal characters were removed.
>>>  - Unsupported tags/properties were removed -like <tt>, <center>,
>>> font, etc.(except the tags related to tables which I'll fix later).
>>>  - HTML5 doctype is set for all files.
>>>  - The <title> is set for all files.
>>>  - <a name="" is replaced by <a id=""
>> Why you replace
>>
>>>  - Copyrights were added to some files.
>>>
>>> Note that I placed a <head> tag before copyright to solve errors like:
>>> "A charset attribute on a meta element found after the first 1024
>>> bytes. Fatal Error: Changing encoding at this point would need
>>> non-streamable behavior"
>>>
>>> specdiff:
>>> http://cr.openjdk.java.net/~serb/8181289/specdiff/overview-summary.html
>>>
>>> Bugs:
>>>     https://bugs.openjdk.java.net/browse/JDK-8182410
>>>     https://bugs.openjdk.java.net/browse/JDK-8183508
>>>     https://bugs.openjdk.java.net/browse/JDK-8181289
>>>
>>> Webrev can be found at:
>>> http://cr.openjdk.java.net/~serb/8181289/webrev.00
>>>
>>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review Request: 8182410, 8183508, 8181289

Sergey Bylokhov
On 24/10/2017 14:56, Semyon Sadetsky wrote:
> On 10/24/2017 12:02 PM, Sergey Bylokhov wrote:
>
>> Hi, Semyon
>> I guess I did not add a new <a> tags, it seems that this tag is used
>> as an anchor everywhere and since it is allowed in html5 its usage was
>> preserved.
> Starting version 9 JDK supports the html5 markup in javadoc, so the
> files need to be updated according to it.

This bug and the fix is a part of those efforts.

> In html5 you need <a> only if
> you want to see hyperlink and your fix is modifying those tags.

<a id="one"></a> is a valid html5 code which can be located at any place
on the page and can be referred using id from any other place. Its usage
were checked using html5 validator.

>
> --Semyon
>>
>> On 23/10/2017 22:08, Semyon Sadetsky wrote:
>>> Hi Sergey,
>>>
>>> I see no reason to have an extra empty anchor tag to set a bookmark.
>>> The id attribute works with any element.
>>>
>>> For example:
>>>
>>>      <a id="Definitions"></a>
>>>      <h3>Definitions</h3>
>>>
>>> should be
>>>
>>>      <h3 id="Definitions">Definitions</h3>
>>>
>>> --Semyon
>>>
>>> On 10/23/2017 02:42 PM, Sergey Bylokhov wrote:
>>>>
>>>> Hello,
>>>> Please review the fix for.
>>>> 8182410: missing 'title' in
>>>> api/javax/swing/plaf/synth/doc-files/componentProperties.html
>>>> 8183508: multi_tsc.html should be updated
>>>> 8181289: Invalid HTML 5 in AWT/Swing docs
>>>>
>>>> Description:
>>>>  - Illegal characters were removed.
>>>>  - Unsupported tags/properties were removed -like <tt>, <center>,
>>>> font, etc.(except the tags related to tables which I'll fix later).
>>>>  - HTML5 doctype is set for all files.
>>>>  - The <title> is set for all files.
>>>>  - <a name="" is replaced by <a id=""
>>> Why you replace
>>>
>>>>  - Copyrights were added to some files.
>>>>
>>>> Note that I placed a <head> tag before copyright to solve errors like:
>>>> "A charset attribute on a meta element found after the first 1024
>>>> bytes. Fatal Error: Changing encoding at this point would need
>>>> non-streamable behavior"
>>>>
>>>> specdiff:
>>>> http://cr.openjdk.java.net/~serb/8181289/specdiff/overview-summary.html
>>>>
>>>> Bugs:
>>>>     https://bugs.openjdk.java.net/browse/JDK-8182410
>>>>     https://bugs.openjdk.java.net/browse/JDK-8183508
>>>>     https://bugs.openjdk.java.net/browse/JDK-8181289
>>>>
>>>> Webrev can be found at:
>>>> http://cr.openjdk.java.net/~serb/8181289/webrev.00
>>>>
>>>
>>
>>
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review Request: 8182410, 8183508, 8181289

Semyon Sadetsky
In reply to this post by Semyon Sadetsky

Hi Jonathan,


On 10/24/2017 03:20 PM, Jonathan Gibbons wrote:

Semyon,

Although id is a global attribute and can be used to identify any node, some browsers do better navigation/scrolling when the id is in an <a> tag.  We have seen poor autoscrolling behavior when the id is an a header tag, such that the header ends up obscured under the navigation bar at the top of the page.
You probably meant heading elements, because "header tag" is something different. Do you have any references those issues reports? Because in html5 the fragment identifiers are the only correct way to have internal document bookmarks [1] [2]. If some browsers do not navigate to fragment identifiers except for <a> element there must be bugs reported that  which will be fixed soon.
The html5 specification is very specific about navigating to the fragment identifier [3]. So, there should no be difference between navigating to "<a id=" or to any other element having id attribute. If you just need an extra vertical space above header you could use css style or <p>, but usage of <a> as an upper margin seems odd since it is a special tag.

--Semyon

[1] https://www.w3schools.com/html/html_links.asp
[2] http://www.html5-tutorials.org/html-basics/links/
[3] https://www.w3.org/TR/html5/browsers.html#scroll-to-fragid


-- Jon


On 10/23/2017 10:08 PM, Semyon Sadetsky wrote:
Hi Sergey,

I see no reason to have an extra empty anchor tag to set a bookmark. The id attribute works with any element.

For example:

    <a id="Definitions"></a>
    <h3>Definitions</h3>

should be

    <h3 id="Definitions">Definitions</h3>

--Semyon

On 10/23/2017 02:42 PM, Sergey Bylokhov wrote:

Hello,
Please review the fix for.
8182410: missing 'title' in api/javax/swing/plaf/synth/doc-files/componentProperties.html
8183508: multi_tsc.html should be updated
8181289: Invalid HTML 5 in AWT/Swing docs

Description:
 - Illegal characters were removed.
 - Unsupported tags/properties were removed -like <tt>, <center>, font, etc.(except the tags related to tables which I'll fix later).
 - HTML5 doctype is set for all files.
 - The <title> is set for all files.
 - <a name="" is replaced by <a id=""
Why you replace

 - Copyrights were added to some files.

Note that I placed a <head> tag before copyright to solve errors like:
"A charset attribute on a meta element found after the first 1024 bytes. Fatal Error: Changing encoding at this point would need non-streamable behavior"

specdiff: http://cr.openjdk.java.net/~serb/8181289/specdiff/overview-summary.html

Bugs:
    https://bugs.openjdk.java.net/browse/JDK-8182410
    https://bugs.openjdk.java.net/browse/JDK-8183508
    https://bugs.openjdk.java.net/browse/JDK-8181289

Webrev can be found at: http://cr.openjdk.java.net/~serb/8181289/webrev.00




Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review Request: 8182410, 8183508, 8181289

Jonathan Gibbons

Semyon,

I read the HTML 5 spec the same as you, and we (on the Javadoc team) started using id on other elements, as well as <a> to provide a target that could be linked to.

However, the pragmatic experience was that the scrolling in some browsers did not completely reveal the element when there was a layered z component involved: the target element sometimes ended up under that layered component. Our experience was that the behavior was fixed when the target identifier was in an <a> element.

So, yes, you can follow the rules, and suggest that it is OK to put id on any element, and use it as a fragment identifier in a link, as given in the spec. Or you can be nice to your readers, and workaround what is probably a display bug in some browsers.

In the case of this review, you were suggesting additional "cleanup" on code that worked. Since there was no bug involved, and thus no inherent need to fix the code, my review feedback is to leave the code alone.  You may choose to insist differently, and I cannot say that what you are suggesting is against the spec; I can just say that we can seen cases where such changes leads to bad visual effects.

-- Jon


On 10/25/17 6:31 PM, Semyon Sadetsky wrote:

Hi Jonathan,


On 10/24/2017 03:20 PM, Jonathan Gibbons wrote:

Semyon,

Although id is a global attribute and can be used to identify any node, some browsers do better navigation/scrolling when the id is in an <a> tag.  We have seen poor autoscrolling behavior when the id is an a header tag, such that the header ends up obscured under the navigation bar at the top of the page.
You probably meant heading elements, because "header tag" is something different. Do you have any references those issues reports? Because in html5 the fragment identifiers are the only correct way to have internal document bookmarks [1] [2]. If some browsers do not navigate to fragment identifiers except for <a> element there must be bugs reported that  which will be fixed soon.
The html5 specification is very specific about navigating to the fragment identifier [3]. So, there should no be difference between navigating to "<a id=" or to any other element having id attribute. If you just need an extra vertical space above header you could use css style or <p>, but usage of <a> as an upper margin seems odd since it is a special tag.

--Semyon

[1] https://www.w3schools.com/html/html_links.asp
[2] http://www.html5-tutorials.org/html-basics/links/
[3] https://www.w3.org/TR/html5/browsers.html#scroll-to-fragid


-- Jon


On 10/23/2017 10:08 PM, Semyon Sadetsky wrote:
Hi Sergey,

I see no reason to have an extra empty anchor tag to set a bookmark. The id attribute works with any element.

For example:

    <a id="Definitions"></a>
    <h3>Definitions</h3>

should be

    <h3 id="Definitions">Definitions</h3>

--Semyon

On 10/23/2017 02:42 PM, Sergey Bylokhov wrote:

Hello,
Please review the fix for.
8182410: missing 'title' in api/javax/swing/plaf/synth/doc-files/componentProperties.html
8183508: multi_tsc.html should be updated
8181289: Invalid HTML 5 in AWT/Swing docs

Description:
 - Illegal characters were removed.
 - Unsupported tags/properties were removed -like <tt>, <center>, font, etc.(except the tags related to tables which I'll fix later).
 - HTML5 doctype is set for all files.
 - The <title> is set for all files.
 - <a name="" is replaced by <a id=""
Why you replace

 - Copyrights were added to some files.

Note that I placed a <head> tag before copyright to solve errors like:
"A charset attribute on a meta element found after the first 1024 bytes. Fatal Error: Changing encoding at this point would need non-streamable behavior"

specdiff: http://cr.openjdk.java.net/~serb/8181289/specdiff/overview-summary.html

Bugs:
    https://bugs.openjdk.java.net/browse/JDK-8182410
    https://bugs.openjdk.java.net/browse/JDK-8183508
    https://bugs.openjdk.java.net/browse/JDK-8181289

Webrev can be found at: http://cr.openjdk.java.net/~serb/8181289/webrev.00





Reply | Threaded
Open this post in threaded view
|

Re: [10] Review Request: 8182410, 8183508, 8181289

Sergey Bylokhov
In reply to this post by Sergey Bylokhov
If there are no other comments about this review, I'll push the first
version(.00) of the fix.

On 23/10/2017 14:42, Sergey Bylokhov wrote:

>
> Hello,
> Please review the fix for.
> 8182410: missing 'title' in
> api/javax/swing/plaf/synth/doc-files/componentProperties.html
> 8183508: multi_tsc.html should be updated
> 8181289: Invalid HTML 5 in AWT/Swing docs
>
> Description:
>   - Illegal characters were removed.
>   - Unsupported tags/properties were removed -like <tt>, <center>, font,
> etc.(except the tags related to tables which I'll fix later).
>   - HTML5 doctype is set for all files.
>   - The <title> is set for all files.
>   - <a name="" is replaced by <a id=""
>   - Copyrights were added to some files.
>
> Note that I placed a <head> tag before copyright to solve errors like:
> "A charset attribute on a meta element found after the first 1024 bytes.
> Fatal Error: Changing encoding at this point would need non-streamable
> behavior"
>
> specdiff:
> http://cr.openjdk.java.net/~serb/8181289/specdiff/overview-summary.html
>
> Bugs:
>      https://bugs.openjdk.java.net/browse/JDK-8182410
>      https://bugs.openjdk.java.net/browse/JDK-8183508
>      https://bugs.openjdk.java.net/browse/JDK-8181289
>
> Webrev can be found at: http://cr.openjdk.java.net/~serb/8181289/webrev.00
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review Request: 8182410, 8183508, 8181289

Semyon Sadetsky
In reply to this post by Jonathan Gibbons

Hi Jon,

This is not only about HTML5 spec, I also hardly can find resources that follow your "<a id=" rule. And I doubt that cross-browser compatibility is important for Javadoc only and others do not care about their readers. So, I asked you for an examples of such workaround or a reference to a bug filed against any browser. Fragment identifiers is too important functionality to let this issue be unnoticeable.

You are correct that there is no bug here. But a bug was absent  before this fix as well. This bug is about following to the HTML5 standards, so let's follow them in full and not to return to this once again. We have a good chance to provide documentation in clean HTML5 after the fix without any workarounds.

--Semyon

On 11/14/2017 09:16 AM, Jonathan Gibbons wrote:

Semyon,

I read the HTML 5 spec the same as you, and we (on the Javadoc team) started using id on other elements, as well as <a> to provide a target that could be linked to.

However, the pragmatic experience was that the scrolling in some browsers did not completely reveal the element when there was a layered z component involved: the target element sometimes ended up under that layered component. Our experience was that the behavior was fixed when the target identifier was in an <a> element.

So, yes, you can follow the rules, and suggest that it is OK to put id on any element, and use it as a fragment identifier in a link, as given in the spec. Or you can be nice to your readers, and workaround what is probably a display bug in some browsers.

In the case of this review, you were suggesting additional "cleanup" on code that worked. Since there was no bug involved, and thus no inherent need to fix the code, my review feedback is to leave the code alone.  You may choose to insist differently, and I cannot say that what you are suggesting is against the spec; I can just say that we can seen cases where such changes leads to bad visual effects.

-- Jon


On 10/25/17 6:31 PM, Semyon Sadetsky wrote:

Hi Jonathan,


On 10/24/2017 03:20 PM, Jonathan Gibbons wrote:

Semyon,

Although id is a global attribute and can be used to identify any node, some browsers do better navigation/scrolling when the id is in an <a> tag.  We have seen poor autoscrolling behavior when the id is an a header tag, such that the header ends up obscured under the navigation bar at the top of the page.
You probably meant heading elements, because "header tag" is something different. Do you have any references those issues reports? Because in html5 the fragment identifiers are the only correct way to have internal document bookmarks [1] [2]. If some browsers do not navigate to fragment identifiers except for <a> element there must be bugs reported that  which will be fixed soon.
The html5 specification is very specific about navigating to the fragment identifier [3]. So, there should no be difference between navigating to "<a id=" or to any other element having id attribute. If you just need an extra vertical space above header you could use css style or <p>, but usage of <a> as an upper margin seems odd since it is a special tag.

--Semyon

[1] https://www.w3schools.com/html/html_links.asp
[2] http://www.html5-tutorials.org/html-basics/links/
[3] https://www.w3.org/TR/html5/browsers.html#scroll-to-fragid


-- Jon


On 10/23/2017 10:08 PM, Semyon Sadetsky wrote:
Hi Sergey,

I see no reason to have an extra empty anchor tag to set a bookmark. The id attribute works with any element.

For example:

    <a id="Definitions"></a>
    <h3>Definitions</h3>

should be

    <h3 id="Definitions">Definitions</h3>

--Semyon

On 10/23/2017 02:42 PM, Sergey Bylokhov wrote:

Hello,
Please review the fix for.
8182410: missing 'title' in api/javax/swing/plaf/synth/doc-files/componentProperties.html
8183508: multi_tsc.html should be updated
8181289: Invalid HTML 5 in AWT/Swing docs

Description:
 - Illegal characters were removed.
 - Unsupported tags/properties were removed -like <tt>, <center>, font, etc.(except the tags related to tables which I'll fix later).
 - HTML5 doctype is set for all files.
 - The <title> is set for all files.
 - <a name="" is replaced by <a id=""
Why you replace

 - Copyrights were added to some files.

Note that I placed a <head> tag before copyright to solve errors like:
"A charset attribute on a meta element found after the first 1024 bytes. Fatal Error: Changing encoding at this point would need non-streamable behavior"

specdiff: http://cr.openjdk.java.net/~serb/8181289/specdiff/overview-summary.html

Bugs:
    https://bugs.openjdk.java.net/browse/JDK-8182410
    https://bugs.openjdk.java.net/browse/JDK-8183508
    https://bugs.openjdk.java.net/browse/JDK-8181289

Webrev can be found at: http://cr.openjdk.java.net/~serb/8181289/webrev.00






Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review Request: 8182410, 8183508, 8181289

Jonathan Gibbons
Semyon,

I have reconstructed a very simple, very artificial example to demo the bug.   This example uses lots of filler text, but while that is artificial, for sake of recreating a demo,  note that the problem first appeared, for real, in real JDK 9 API documentation with extended doc comments, and that as a result, we followed the advice I have been trying to give you.

See the toy API bundle here:
http://cr.openjdk.java.net/~jjg/semyon/api/overview-summary.html

There are two modules, modA and modB.  Both have huge long doc comments, with a heading at the top and a link at the bottom.

In modA, the anchor is of the form <h1 id="head">.  In modB, the anchor is of the form <a id="head">.

In each of these files, scroll to the end of the comment, and look for a link, called "link", at the bottom of the page.  In both cases, the page scrolls so that the heading is near the top of the browser window, but in one case it is hidden under the javadoc navbar, and in the other case, it is clearly visible, below the javadoc navbar.

This is the difference in behavior that I can been trying to describe to you. I'm using Ubuntu 14.04 with Firefox 38, but I'm not the only one to have seen this effect.  I don't know whether you will get the same effect in your browser, but the fact that there is a reasonable OS/browser combo that demonstrates the problem is enough of a reason to avoid provoking the problem unnecessarily.   If you don't see the problem on your browser, but want to see it in mine, I see you are in SCA22, so drop by my office for a demo.

I'll leave it to the AWT team to decide what to do about this bug/review. I still recommend updating what is necessary to fix issues, and not otherwise changing the doc comments unnecessarily, and not changing them in a way to provoke this bad behavior.

-- Jon




On 11/22/2017 12:10 PM, Semyon Sadetsky wrote:

Hi Jon,

This is not only about HTML5 spec, I also hardly can find resources that follow your "<a id=" rule. And I doubt that cross-browser compatibility is important for Javadoc only and others do not care about their readers. So, I asked you for an examples of such workaround or a reference to a bug filed against any browser. Fragment identifiers is too important functionality to let this issue be unnoticeable.

You are correct that there is no bug here. But a bug was absent  before this fix as well. This bug is about following to the HTML5 standards, so let's follow them in full and not to return to this once again. We have a good chance to provide documentation in clean HTML5 after the fix without any workarounds.

--Semyon

On 11/14/2017 09:16 AM, Jonathan Gibbons wrote:

Semyon,

I read the HTML 5 spec the same as you, and we (on the Javadoc team) started using id on other elements, as well as <a> to provide a target that could be linked to.

However, the pragmatic experience was that the scrolling in some browsers did not completely reveal the element when there was a layered z component involved: the target element sometimes ended up under that layered component. Our experience was that the behavior was fixed when the target identifier was in an <a> element.

So, yes, you can follow the rules, and suggest that it is OK to put id on any element, and use it as a fragment identifier in a link, as given in the spec. Or you can be nice to your readers, and workaround what is probably a display bug in some browsers.

In the case of this review, you were suggesting additional "cleanup" on code that worked. Since there was no bug involved, and thus no inherent need to fix the code, my review feedback is to leave the code alone.  You may choose to insist differently, and I cannot say that what you are suggesting is against the spec; I can just say that we can seen cases where such changes leads to bad visual effects.

-- Jon


On 10/25/17 6:31 PM, Semyon Sadetsky wrote:

Hi Jonathan,


On 10/24/2017 03:20 PM, Jonathan Gibbons wrote:

Semyon,

Although id is a global attribute and can be used to identify any node, some browsers do better navigation/scrolling when the id is in an <a> tag.  We have seen poor autoscrolling behavior when the id is an a header tag, such that the header ends up obscured under the navigation bar at the top of the page.
You probably meant heading elements, because "header tag" is something different. Do you have any references those issues reports? Because in html5 the fragment identifiers are the only correct way to have internal document bookmarks [1] [2]. If some browsers do not navigate to fragment identifiers except for <a> element there must be bugs reported that  which will be fixed soon.
The html5 specification is very specific about navigating to the fragment identifier [3]. So, there should no be difference between navigating to "<a id=" or to any other element having id attribute. If you just need an extra vertical space above header you could use css style or <p>, but usage of <a> as an upper margin seems odd since it is a special tag.

--Semyon

[1] https://www.w3schools.com/html/html_links.asp
[2] http://www.html5-tutorials.org/html-basics/links/
[3] https://www.w3.org/TR/html5/browsers.html#scroll-to-fragid


-- Jon


On 10/23/2017 10:08 PM, Semyon Sadetsky wrote:
Hi Sergey,

I see no reason to have an extra empty anchor tag to set a bookmark. The id attribute works with any element.

For example:

    <a id="Definitions"></a>
    <h3>Definitions</h3>

should be

    <h3 id="Definitions">Definitions</h3>

--Semyon

On 10/23/2017 02:42 PM, Sergey Bylokhov wrote:

Hello,
Please review the fix for.
8182410: missing 'title' in api/javax/swing/plaf/synth/doc-files/componentProperties.html
8183508: multi_tsc.html should be updated
8181289: Invalid HTML 5 in AWT/Swing docs

Description:
 - Illegal characters were removed.
 - Unsupported tags/properties were removed -like <tt>, <center>, font, etc.(except the tags related to tables which I'll fix later).
 - HTML5 doctype is set for all files.
 - The <title> is set for all files.
 - <a name="" is replaced by <a id=""
Why you replace

 - Copyrights were added to some files.

Note that I placed a <head> tag before copyright to solve errors like:
"A charset attribute on a meta element found after the first 1024 bytes. Fatal Error: Changing encoding at this point would need non-streamable behavior"

specdiff: http://cr.openjdk.java.net/~serb/8181289/specdiff/overview-summary.html

Bugs:
    https://bugs.openjdk.java.net/browse/JDK-8182410
    https://bugs.openjdk.java.net/browse/JDK-8183508
    https://bugs.openjdk.java.net/browse/JDK-8181289

Webrev can be found at: http://cr.openjdk.java.net/~serb/8181289/webrev.00







Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review Request: 8182410, 8183508, 8181289

Jonathan Gibbons
Semyon,

Using the files I previously posted, I confirm that I see the same display problem on a Mac, using the latest OS (High Sierra) and Safari. I also see the problem on the same Mac, with Firefox 55.0.2

-- Jon

On 11/22/2017 02:53 PM, Jonathan Gibbons wrote:
Semyon,

I have reconstructed a very simple, very artificial example to demo the bug.   This example uses lots of filler text, but while that is artificial, for sake of recreating a demo,  note that the problem first appeared, for real, in real JDK 9 API documentation with extended doc comments, and that as a result, we followed the advice I have been trying to give you.

See the toy API bundle here:
http://cr.openjdk.java.net/~jjg/semyon/api/overview-summary.html

There are two modules, modA and modB.  Both have huge long doc comments, with a heading at the top and a link at the bottom.

In modA, the anchor is of the form <h1 id="head">.  In modB, the anchor is of the form <a id="head">.

In each of these files, scroll to the end of the comment, and look for a link, called "link", at the bottom of the page.  In both cases, the page scrolls so that the heading is near the top of the browser window, but in one case it is hidden under the javadoc navbar, and in the other case, it is clearly visible, below the javadoc navbar.

This is the difference in behavior that I can been trying to describe to you. I'm using Ubuntu 14.04 with Firefox 38, but I'm not the only one to have seen this effect.  I don't know whether you will get the same effect in your browser, but the fact that there is a reasonable OS/browser combo that demonstrates the problem is enough of a reason to avoid provoking the problem unnecessarily.   If you don't see the problem on your browser, but want to see it in mine, I see you are in SCA22, so drop by my office for a demo.

I'll leave it to the AWT team to decide what to do about this bug/review. I still recommend updating what is necessary to fix issues, and not otherwise changing the doc comments unnecessarily, and not changing them in a way to provoke this bad behavior.

-- Jon




On 11/22/2017 12:10 PM, Semyon Sadetsky wrote:

Hi Jon,

This is not only about HTML5 spec, I also hardly can find resources that follow your "<a id=" rule. And I doubt that cross-browser compatibility is important for Javadoc only and others do not care about their readers. So, I asked you for an examples of such workaround or a reference to a bug filed against any browser. Fragment identifiers is too important functionality to let this issue be unnoticeable.

You are correct that there is no bug here. But a bug was absent  before this fix as well. This bug is about following to the HTML5 standards, so let's follow them in full and not to return to this once again. We have a good chance to provide documentation in clean HTML5 after the fix without any workarounds.

--Semyon

On 11/14/2017 09:16 AM, Jonathan Gibbons wrote:

Semyon,

I read the HTML 5 spec the same as you, and we (on the Javadoc team) started using id on other elements, as well as <a> to provide a target that could be linked to.

However, the pragmatic experience was that the scrolling in some browsers did not completely reveal the element when there was a layered z component involved: the target element sometimes ended up under that layered component. Our experience was that the behavior was fixed when the target identifier was in an <a> element.

So, yes, you can follow the rules, and suggest that it is OK to put id on any element, and use it as a fragment identifier in a link, as given in the spec. Or you can be nice to your readers, and workaround what is probably a display bug in some browsers.

In the case of this review, you were suggesting additional "cleanup" on code that worked. Since there was no bug involved, and thus no inherent need to fix the code, my review feedback is to leave the code alone.  You may choose to insist differently, and I cannot say that what you are suggesting is against the spec; I can just say that we can seen cases where such changes leads to bad visual effects.

-- Jon


On 10/25/17 6:31 PM, Semyon Sadetsky wrote:

Hi Jonathan,


On 10/24/2017 03:20 PM, Jonathan Gibbons wrote:

Semyon,

Although id is a global attribute and can be used to identify any node, some browsers do better navigation/scrolling when the id is in an <a> tag.  We have seen poor autoscrolling behavior when the id is an a header tag, such that the header ends up obscured under the navigation bar at the top of the page.
You probably meant heading elements, because "header tag" is something different. Do you have any references those issues reports? Because in html5 the fragment identifiers are the only correct way to have internal document bookmarks [1] [2]. If some browsers do not navigate to fragment identifiers except for <a> element there must be bugs reported that  which will be fixed soon.
The html5 specification is very specific about navigating to the fragment identifier [3]. So, there should no be difference between navigating to "<a id=" or to any other element having id attribute. If you just need an extra vertical space above header you could use css style or <p>, but usage of <a> as an upper margin seems odd since it is a special tag.

--Semyon

[1] https://www.w3schools.com/html/html_links.asp
[2] http://www.html5-tutorials.org/html-basics/links/
[3] https://www.w3.org/TR/html5/browsers.html#scroll-to-fragid


-- Jon


On 10/23/2017 10:08 PM, Semyon Sadetsky wrote:
Hi Sergey,

I see no reason to have an extra empty anchor tag to set a bookmark. The id attribute works with any element.

For example:

    <a id="Definitions"></a>
    <h3>Definitions</h3>

should be

    <h3 id="Definitions">Definitions</h3>

--Semyon

On 10/23/2017 02:42 PM, Sergey Bylokhov wrote:

Hello,
Please review the fix for.
8182410: missing 'title' in api/javax/swing/plaf/synth/doc-files/componentProperties.html
8183508: multi_tsc.html should be updated
8181289: Invalid HTML 5 in AWT/Swing docs

Description:
 - Illegal characters were removed.
 - Unsupported tags/properties were removed -like <tt>, <center>, font, etc.(except the tags related to tables which I'll fix later).
 - HTML5 doctype is set for all files.
 - The <title> is set for all files.
 - <a name="" is replaced by <a id=""
Why you replace

 - Copyrights were added to some files.

Note that I placed a <head> tag before copyright to solve errors like:
"A charset attribute on a meta element found after the first 1024 bytes. Fatal Error: Changing encoding at this point would need non-streamable behavior"

specdiff: http://cr.openjdk.java.net/~serb/8181289/specdiff/overview-summary.html

Bugs:
    https://bugs.openjdk.java.net/browse/JDK-8182410
    https://bugs.openjdk.java.net/browse/JDK-8183508
    https://bugs.openjdk.java.net/browse/JDK-8181289

Webrev can be found at: http://cr.openjdk.java.net/~serb/8181289/webrev.00








Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review Request: 8182410, 8183508, 8181289

Kevin Rushforth
I can confirm the same behavior that Jon sees with Firefox 57 on Windows 7.

The link scrolls to the right place with the <a id="..."> link and does not with the <h1 id="..."> link.

-- Kevin


Jonathan Gibbons wrote:
Semyon,

Using the files I previously posted, I confirm that I see the same display problem on a Mac, using the latest OS (High Sierra) and Safari. I also see the problem on the same Mac, with Firefox 55.0.2

-- Jon

On 11/22/2017 02:53 PM, Jonathan Gibbons wrote:
Semyon,

I have reconstructed a very simple, very artificial example to demo the bug.   This example uses lots of filler text, but while that is artificial, for sake of recreating a demo,  note that the problem first appeared, for real, in real JDK 9 API documentation with extended doc comments, and that as a result, we followed the advice I have been trying to give you.

See the toy API bundle here:
http://cr.openjdk.java.net/~jjg/semyon/api/overview-summary.html

There are two modules, modA and modB.  Both have huge long doc comments, with a heading at the top and a link at the bottom.

In modA, the anchor is of the form <h1 id="head">.  In modB, the anchor is of the form <a id="head">.

In each of these files, scroll to the end of the comment, and look for a link, called "link", at the bottom of the page.  In both cases, the page scrolls so that the heading is near the top of the browser window, but in one case it is hidden under the javadoc navbar, and in the other case, it is clearly visible, below the javadoc navbar.

This is the difference in behavior that I can been trying to describe to you. I'm using Ubuntu 14.04 with Firefox 38, but I'm not the only one to have seen this effect.  I don't know whether you will get the same effect in your browser, but the fact that there is a reasonable OS/browser combo that demonstrates the problem is enough of a reason to avoid provoking the problem unnecessarily.   If you don't see the problem on your browser, but want to see it in mine, I see you are in SCA22, so drop by my office for a demo.

I'll leave it to the AWT team to decide what to do about this bug/review. I still recommend updating what is necessary to fix issues, and not otherwise changing the doc comments unnecessarily, and not changing them in a way to provoke this bad behavior.

-- Jon




On 11/22/2017 12:10 PM, Semyon Sadetsky wrote:

Hi Jon,

This is not only about HTML5 spec, I also hardly can find resources that follow your "<a id=" rule. And I doubt that cross-browser compatibility is important for Javadoc only and others do not care about their readers. So, I asked you for an examples of such workaround or a reference to a bug filed against any browser. Fragment identifiers is too important functionality to let this issue be unnoticeable.

You are correct that there is no bug here. But a bug was absent  before this fix as well. This bug is about following to the HTML5 standards, so let's follow them in full and not to return to this once again. We have a good chance to provide documentation in clean HTML5 after the fix without any workarounds.

--Semyon

On 11/14/2017 09:16 AM, Jonathan Gibbons wrote:

Semyon,

I read the HTML 5 spec the same as you, and we (on the Javadoc team) started using id on other elements, as well as <a> to provide a target that could be linked to.

However, the pragmatic experience was that the scrolling in some browsers did not completely reveal the element when there was a layered z component involved: the target element sometimes ended up under that layered component. Our experience was that the behavior was fixed when the target identifier was in an <a> element.

So, yes, you can follow the rules, and suggest that it is OK to put id on any element, and use it as a fragment identifier in a link, as given in the spec. Or you can be nice to your readers, and workaround what is probably a display bug in some browsers.

In the case of this review, you were suggesting additional "cleanup" on code that worked. Since there was no bug involved, and thus no inherent need to fix the code, my review feedback is to leave the code alone.  You may choose to insist differently, and I cannot say that what you are suggesting is against the spec; I can just say that we can seen cases where such changes leads to bad visual effects.

-- Jon


On 10/25/17 6:31 PM, Semyon Sadetsky wrote:

Hi Jonathan,


On 10/24/2017 03:20 PM, Jonathan Gibbons wrote:

Semyon,

Although id is a global attribute and can be used to identify any node, some browsers do better navigation/scrolling when the id is in an <a> tag.  We have seen poor autoscrolling behavior when the id is an a header tag, such that the header ends up obscured under the navigation bar at the top of the page.
You probably meant heading elements, because "header tag" is something different. Do you have any references those issues reports? Because in html5 the fragment identifiers are the only correct way to have internal document bookmarks [1] [2]. If some browsers do not navigate to fragment identifiers except for <a> element there must be bugs reported that  which will be fixed soon.
The html5 specification is very specific about navigating to the fragment identifier [3]. So, there should no be difference between navigating to "<a id=" or to any other element having id attribute. If you just need an extra vertical space above header you could use css style or <p>, but usage of <a> as an upper margin seems odd since it is a special tag.

--Semyon

[1] https://www.w3schools.com/html/html_links.asp
[2] http://www.html5-tutorials.org/html-basics/links/
[3] https://www.w3.org/TR/html5/browsers.html#scroll-to-fragid


-- Jon


On 10/23/2017 10:08 PM, Semyon Sadetsky wrote:
Hi Sergey,

I see no reason to have an extra empty anchor tag to set a bookmark. The id attribute works with any element.

For example:

    <a id="Definitions"></a>
    <h3>Definitions</h3>

should be

    <h3 id="Definitions">Definitions</h3>

--Semyon

On 10/23/2017 02:42 PM, Sergey Bylokhov wrote:

Hello,
Please review the fix for.
8182410: missing 'title' in api/javax/swing/plaf/synth/doc-files/componentProperties.html
8183508: multi_tsc.html should be updated
8181289: Invalid HTML 5 in AWT/Swing docs

Description:
 - Illegal characters were removed.
 - Unsupported tags/properties were removed -like <tt>, <center>, font, etc.(except the tags related to tables which I'll fix later).
 - HTML5 doctype is set for all files.
 - The <title> is set for all files.
 - <a name="" is replaced by <a id=""
Why you replace

 - Copyrights were added to some files.

Note that I placed a <head> tag before copyright to solve errors like:
"A charset attribute on a meta element found after the first 1024 bytes. Fatal Error: Changing encoding at this point would need non-streamable behavior"

specdiff: http://cr.openjdk.java.net/~serb/8181289/specdiff/overview-summary.html

Bugs:
    https://bugs.openjdk.java.net/browse/JDK-8182410
    https://bugs.openjdk.java.net/browse/JDK-8183508
    https://bugs.openjdk.java.net/browse/JDK-8181289

Webrev can be found at: http://cr.openjdk.java.net/~serb/8181289/webrev.00








Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review Request: 8182410, 8183508, 8181289

Semyon Sadetsky
In reply to this post by Jonathan Gibbons

Jon,

This is because you have fixed page header. For me it works equally in all browsers. I see no discrepancy between Chrome and Firefox on my Linux platform. I believe that the stylesheet.css you have in those examples does the magic :

a[name]:before, a[name]:target, a[id]:before, a[id]:target {
    content: "";
    display: inline-block;
    position: relative;
    padding-top: 129px;
    margin-top: -129px;
}

so nothing specific comes from browser or "<a id=" it is just a special margin/padding is set for a[id] as I suspect at the beginning. This css rule is well known solution for the problem.

I think the next link may help you

http://nicolasgallagher.com/jump-links-and-viewport-positioning/demo/

--Semyon


On 11/22/2017 02:53 PM, Jonathan Gibbons wrote:
Semyon,

I have reconstructed a very simple, very artificial example to demo the bug.   This example uses lots of filler text, but while that is artificial, for sake of recreating a demo,  note that the problem first appeared, for real, in real JDK 9 API documentation with extended doc comments, and that as a result, we followed the advice I have been trying to give you.

See the toy API bundle here:
http://cr.openjdk.java.net/~jjg/semyon/api/overview-summary.html

There are two modules, modA and modB.  Both have huge long doc comments, with a heading at the top and a link at the bottom.

In modA, the anchor is of the form <h1 id="head">.  In modB, the anchor is of the form <a id="head">.

In each of these files, scroll to the end of the comment, and look for a link, called "link", at the bottom of the page.  In both cases, the page scrolls so that the heading is near the top of the browser window, but in one case it is hidden under the javadoc navbar, and in the other case, it is clearly visible, below the javadoc navbar.

This is the difference in behavior that I can been trying to describe to you. I'm using Ubuntu 14.04 with Firefox 38, but I'm not the only one to have seen this effect.  I don't know whether you will get the same effect in your browser, but the fact that there is a reasonable OS/browser combo that demonstrates the problem is enough of a reason to avoid provoking the problem unnecessarily.   If you don't see the problem on your browser, but want to see it in mine, I see you are in SCA22, so drop by my office for a demo.

I'll leave it to the AWT team to decide what to do about this bug/review. I still recommend updating what is necessary to fix issues, and not otherwise changing the doc comments unnecessarily, and not changing them in a way to provoke this bad behavior.

-- Jon




On 11/22/2017 12:10 PM, Semyon Sadetsky wrote:

Hi Jon,

This is not only about HTML5 spec, I also hardly can find resources that follow your "<a id=" rule. And I doubt that cross-browser compatibility is important for Javadoc only and others do not care about their readers. So, I asked you for an examples of such workaround or a reference to a bug filed against any browser. Fragment identifiers is too important functionality to let this issue be unnoticeable.

You are correct that there is no bug here. But a bug was absent  before this fix as well. This bug is about following to the HTML5 standards, so let's follow them in full and not to return to this once again. We have a good chance to provide documentation in clean HTML5 after the fix without any workarounds.

--Semyon

On 11/14/2017 09:16 AM, Jonathan Gibbons wrote:

Semyon,

I read the HTML 5 spec the same as you, and we (on the Javadoc team) started using id on other elements, as well as <a> to provide a target that could be linked to.

However, the pragmatic experience was that the scrolling in some browsers did not completely reveal the element when there was a layered z component involved: the target element sometimes ended up under that layered component. Our experience was that the behavior was fixed when the target identifier was in an <a> element.

So, yes, you can follow the rules, and suggest that it is OK to put id on any element, and use it as a fragment identifier in a link, as given in the spec. Or you can be nice to your readers, and workaround what is probably a display bug in some browsers.

In the case of this review, you were suggesting additional "cleanup" on code that worked. Since there was no bug involved, and thus no inherent need to fix the code, my review feedback is to leave the code alone.  You may choose to insist differently, and I cannot say that what you are suggesting is against the spec; I can just say that we can seen cases where such changes leads to bad visual effects.

-- Jon


On 10/25/17 6:31 PM, Semyon Sadetsky wrote:

Hi Jonathan,


On 10/24/2017 03:20 PM, Jonathan Gibbons wrote:

Semyon,

Although id is a global attribute and can be used to identify any node, some browsers do better navigation/scrolling when the id is in an <a> tag.  We have seen poor autoscrolling behavior when the id is an a header tag, such that the header ends up obscured under the navigation bar at the top of the page.
You probably meant heading elements, because "header tag" is something different. Do you have any references those issues reports? Because in html5 the fragment identifiers are the only correct way to have internal document bookmarks [1] [2]. If some browsers do not navigate to fragment identifiers except for <a> element there must be bugs reported that  which will be fixed soon.
The html5 specification is very specific about navigating to the fragment identifier [3]. So, there should no be difference between navigating to "<a id=" or to any other element having id attribute. If you just need an extra vertical space above header you could use css style or <p>, but usage of <a> as an upper margin seems odd since it is a special tag.

--Semyon

[1] https://www.w3schools.com/html/html_links.asp
[2] http://www.html5-tutorials.org/html-basics/links/
[3] https://www.w3.org/TR/html5/browsers.html#scroll-to-fragid


-- Jon


On 10/23/2017 10:08 PM, Semyon Sadetsky wrote:
Hi Sergey,

I see no reason to have an extra empty anchor tag to set a bookmark. The id attribute works with any element.

For example:

    <a id="Definitions"></a>
    <h3>Definitions</h3>

should be

    <h3 id="Definitions">Definitions</h3>

--Semyon

On 10/23/2017 02:42 PM, Sergey Bylokhov wrote:

Hello,
Please review the fix for.
8182410: missing 'title' in api/javax/swing/plaf/synth/doc-files/componentProperties.html
8183508: multi_tsc.html should be updated
8181289: Invalid HTML 5 in AWT/Swing docs

Description:
 - Illegal characters were removed.
 - Unsupported tags/properties were removed -like <tt>, <center>, font, etc.(except the tags related to tables which I'll fix later).
 - HTML5 doctype is set for all files.
 - The <title> is set for all files.
 - <a name="" is replaced by <a id=""
Why you replace

 - Copyrights were added to some files.

Note that I placed a <head> tag before copyright to solve errors like:
"A charset attribute on a meta element found after the first 1024 bytes. Fatal Error: Changing encoding at this point would need non-streamable behavior"

specdiff: http://cr.openjdk.java.net/~serb/8181289/specdiff/overview-summary.html

Bugs:
    https://bugs.openjdk.java.net/browse/JDK-8182410
    https://bugs.openjdk.java.net/browse/JDK-8183508
    https://bugs.openjdk.java.net/browse/JDK-8181289

Webrev can be found at: http://cr.openjdk.java.net/~serb/8181289/webrev.00








Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review Request: 8182410, 8183508, 8181289

Jonathan Gibbons

Semyon,

You may indeed have explained why the behavior as it is, but we cannot and should not link this review with changes to the javadoc stylesheets, when the specific changes in the review are gratuitous and not necessary in the first place.

Yes, we may separately, and later, look at how the javadoc manages the header. Until then, I recommend that we stay within guidelines that are fully conformant HTML5, and without visual issues with the existing stylesheet.

So, I want to end this back and forth. I've spent enough time on this. I've given my review feedback, which remains to not introduce changes which may cause visual issues. If you and the  AWT team want to proceed with those changes, I'm done.

-- Jon

On 11/22/17 7:46 PM, Semyon Sadetsky wrote:

Jon,

This is because you have fixed page header. For me it works equally in all browsers. I see no discrepancy between Chrome and Firefox on my Linux platform. I believe that the stylesheet.css you have in those examples does the magic :

a[name]:before, a[name]:target, a[id]:before, a[id]:target {
    content: "";
    display: inline-block;
    position: relative;
    padding-top: 129px;
    margin-top: -129px;
}

so nothing specific comes from browser or "<a id=" it is just a special margin/padding is set for a[id] as I suspect at the beginning. This css rule is well known solution for the problem.

I think the next link may help you

http://nicolasgallagher.com/jump-links-and-viewport-positioning/demo/

--Semyon


On 11/22/2017 02:53 PM, Jonathan Gibbons wrote:
Semyon,

I have reconstructed a very simple, very artificial example to demo the bug.   This example uses lots of filler text, but while that is artificial, for sake of recreating a demo,  note that the problem first appeared, for real, in real JDK 9 API documentation with extended doc comments, and that as a result, we followed the advice I have been trying to give you.

See the toy API bundle here:
http://cr.openjdk.java.net/~jjg/semyon/api/overview-summary.html

There are two modules, modA and modB.  Both have huge long doc comments, with a heading at the top and a link at the bottom.

In modA, the anchor is of the form <h1 id="head">.  In modB, the anchor is of the form <a id="head">.

In each of these files, scroll to the end of the comment, and look for a link, called "link", at the bottom of the page.  In both cases, the page scrolls so that the heading is near the top of the browser window, but in one case it is hidden under the javadoc navbar, and in the other case, it is clearly visible, below the javadoc navbar.

This is the difference in behavior that I can been trying to describe to you. I'm using Ubuntu 14.04 with Firefox 38, but I'm not the only one to have seen this effect.  I don't know whether you will get the same effect in your browser, but the fact that there is a reasonable OS/browser combo that demonstrates the problem is enough of a reason to avoid provoking the problem unnecessarily.   If you don't see the problem on your browser, but want to see it in mine, I see you are in SCA22, so drop by my office for a demo.

I'll leave it to the AWT team to decide what to do about this bug/review. I still recommend updating what is necessary to fix issues, and not otherwise changing the doc comments unnecessarily, and not changing them in a way to provoke this bad behavior.

-- Jon




On 11/22/2017 12:10 PM, Semyon Sadetsky wrote:

Hi Jon,

This is not only about HTML5 spec, I also hardly can find resources that follow your "<a id=" rule. And I doubt that cross-browser compatibility is important for Javadoc only and others do not care about their readers. So, I asked you for an examples of such workaround or a reference to a bug filed against any browser. Fragment identifiers is too important functionality to let this issue be unnoticeable.

You are correct that there is no bug here. But a bug was absent  before this fix as well. This bug is about following to the HTML5 standards, so let's follow them in full and not to return to this once again. We have a good chance to provide documentation in clean HTML5 after the fix without any workarounds.

--Semyon

On 11/14/2017 09:16 AM, Jonathan Gibbons wrote:

Semyon,

I read the HTML 5 spec the same as you, and we (on the Javadoc team) started using id on other elements, as well as <a> to provide a target that could be linked to.

However, the pragmatic experience was that the scrolling in some browsers did not completely reveal the element when there was a layered z component involved: the target element sometimes ended up under that layered component. Our experience was that the behavior was fixed when the target identifier was in an <a> element.

So, yes, you can follow the rules, and suggest that it is OK to put id on any element, and use it as a fragment identifier in a link, as given in the spec. Or you can be nice to your readers, and workaround what is probably a display bug in some browsers.

In the case of this review, you were suggesting additional "cleanup" on code that worked. Since there was no bug involved, and thus no inherent need to fix the code, my review feedback is to leave the code alone.  You may choose to insist differently, and I cannot say that what you are suggesting is against the spec; I can just say that we can seen cases where such changes leads to bad visual effects.

-- Jon


On 10/25/17 6:31 PM, Semyon Sadetsky wrote:

Hi Jonathan,


On 10/24/2017 03:20 PM, Jonathan Gibbons wrote:

Semyon,

Although id is a global attribute and can be used to identify any node, some browsers do better navigation/scrolling when the id is in an <a> tag.  We have seen poor autoscrolling behavior when the id is an a header tag, such that the header ends up obscured under the navigation bar at the top of the page.
You probably meant heading elements, because "header tag" is something different. Do you have any references those issues reports? Because in html5 the fragment identifiers are the only correct way to have internal document bookmarks [1] [2]. If some browsers do not navigate to fragment identifiers except for <a> element there must be bugs reported that  which will be fixed soon.
The html5 specification is very specific about navigating to the fragment identifier [3]. So, there should no be difference between navigating to "<a id=" or to any other element having id attribute. If you just need an extra vertical space above header you could use css style or <p>, but usage of <a> as an upper margin seems odd since it is a special tag.

--Semyon

[1] https://www.w3schools.com/html/html_links.asp
[2] http://www.html5-tutorials.org/html-basics/links/
[3] https://www.w3.org/TR/html5/browsers.html#scroll-to-fragid


-- Jon


On 10/23/2017 10:08 PM, Semyon Sadetsky wrote:
Hi Sergey,

I see no reason to have an extra empty anchor tag to set a bookmark. The id attribute works with any element.

For example:

    <a id="Definitions"></a>
    <h3>Definitions</h3>

should be

    <h3 id="Definitions">Definitions</h3>

--Semyon

On 10/23/2017 02:42 PM, Sergey Bylokhov wrote:

Hello,
Please review the fix for.
8182410: missing 'title' in api/javax/swing/plaf/synth/doc-files/componentProperties.html
8183508: multi_tsc.html should be updated
8181289: Invalid HTML 5 in AWT/Swing docs

Description:
 - Illegal characters were removed.
 - Unsupported tags/properties were removed -like <tt>, <center>, font, etc.(except the tags related to tables which I'll fix later).
 - HTML5 doctype is set for all files.
 - The <title> is set for all files.
 - <a name="" is replaced by <a id=""
Why you replace

 - Copyrights were added to some files.

Note that I placed a <head> tag before copyright to solve errors like:
"A charset attribute on a meta element found after the first 1024 bytes. Fatal Error: Changing encoding at this point would need non-streamable behavior"

specdiff: http://cr.openjdk.java.net/~serb/8181289/specdiff/overview-summary.html

Bugs:
    https://bugs.openjdk.java.net/browse/JDK-8182410
    https://bugs.openjdk.java.net/browse/JDK-8183508
    https://bugs.openjdk.java.net/browse/JDK-8181289

Webrev can be found at: http://cr.openjdk.java.net/~serb/8181289/webrev.00









Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> [10] Review Request: 8182410, 8183508, 8181289

Semyon Sadetsky

Jon,

You shouldn't use empty <a> tag for margins. It looks odd. You can add the same css rule for appropriate element. I don't understand which visual issues you mean.

--Semyon

On 11/22/2017 07:58 PM, Jonathan Gibbons wrote:

Semyon,

You may indeed have explained why the behavior as it is, but we cannot and should not link this review with changes to the javadoc stylesheets, when the specific changes in the review are gratuitous and not necessary in the first place.

Yes, we may separately, and later, look at how the javadoc manages the header. Until then, I recommend that we stay within guidelines that are fully conformant HTML5, and without visual issues with the existing stylesheet.

So, I want to end this back and forth. I've spent enough time on this. I've given my review feedback, which remains to not introduce changes which may cause visual issues. If you and the  AWT team want to proceed with those changes, I'm done.

-- Jon

On 11/22/17 7:46 PM, Semyon Sadetsky wrote:

Jon,

This is because you have fixed page header. For me it works equally in all browsers. I see no discrepancy between Chrome and Firefox on my Linux platform. I believe that the stylesheet.css you have in those examples does the magic :

a[name]:before, a[name]:target, a[id]:before, a[id]:target {
    content: "";
    display: inline-block;
    position: relative;
    padding-top: 129px;
    margin-top: -129px;
}

so nothing specific comes from browser or "<a id=" it is just a special margin/padding is set for a[id] as I suspect at the beginning. This css rule is well known solution for the problem.

I think the next link may help you

http://nicolasgallagher.com/jump-links-and-viewport-positioning/demo/

--Semyon


On 11/22/2017 02:53 PM, Jonathan Gibbons wrote:
Semyon,

I have reconstructed a very simple, very artificial example to demo the bug.   This example uses lots of filler text, but while that is artificial, for sake of recreating a demo,  note that the problem first appeared, for real, in real JDK 9 API documentation with extended doc comments, and that as a result, we followed the advice I have been trying to give you.

See the toy API bundle here:
http://cr.openjdk.java.net/~jjg/semyon/api/overview-summary.html

There are two modules, modA and modB.  Both have huge long doc comments, with a heading at the top and a link at the bottom.

In modA, the anchor is of the form <h1 id="head">.  In modB, the anchor is of the form <a id="head">.

In each of these files, scroll to the end of the comment, and look for a link, called "link", at the bottom of the page.  In both cases, the page scrolls so that the heading is near the top of the browser window, but in one case it is hidden under the javadoc navbar, and in the other case, it is clearly visible, below the javadoc navbar.

This is the difference in behavior that I can been trying to describe to you. I'm using Ubuntu 14.04 with Firefox 38, but I'm not the only one to have seen this effect.  I don't know whether you will get the same effect in your browser, but the fact that there is a reasonable OS/browser combo that demonstrates the problem is enough of a reason to avoid provoking the problem unnecessarily.   If you don't see the problem on your browser, but want to see it in mine, I see you are in SCA22, so drop by my office for a demo.

I'll leave it to the AWT team to decide what to do about this bug/review. I still recommend updating what is necessary to fix issues, and not otherwise changing the doc comments unnecessarily, and not changing them in a way to provoke this bad behavior.

-- Jon




On 11/22/2017 12:10 PM, Semyon Sadetsky wrote:

Hi Jon,

This is not only about HTML5 spec, I also hardly can find resources that follow your "<a id=" rule. And I doubt that cross-browser compatibility is important for Javadoc only and others do not care about their readers. So, I asked you for an examples of such workaround or a reference to a bug filed against any browser. Fragment identifiers is too important functionality to let this issue be unnoticeable.

You are correct that there is no bug here. But a bug was absent  before this fix as well. This bug is about following to the HTML5 standards, so let's follow them in full and not to return to this once again. We have a good chance to provide documentation in clean HTML5 after the fix without any workarounds.

--Semyon

On 11/14/2017 09:16 AM, Jonathan Gibbons wrote:

Semyon,

I read the HTML 5 spec the same as you, and we (on the Javadoc team) started using id on other elements, as well as <a> to provide a target that could be linked to.

However, the pragmatic experience was that the scrolling in some browsers did not completely reveal the element when there was a layered z component involved: the target element sometimes ended up under that layered component. Our experience was that the behavior was fixed when the target identifier was in an <a> element.

So, yes, you can follow the rules, and suggest that it is OK to put id on any element, and use it as a fragment identifier in a link, as given in the spec. Or you can be nice to your readers, and workaround what is probably a display bug in some browsers.

In the case of this review, you were suggesting additional "cleanup" on code that worked. Since there was no bug involved, and thus no inherent need to fix the code, my review feedback is to leave the code alone.  You may choose to insist differently, and I cannot say that what you are suggesting is against the spec; I can just say that we can seen cases where such changes leads to bad visual effects.

-- Jon


On 10/25/17 6:31 PM, Semyon Sadetsky wrote:

Hi Jonathan,


On 10/24/2017 03:20 PM, Jonathan Gibbons wrote:

Semyon,

Although id is a global attribute and can be used to identify any node, some browsers do better navigation/scrolling when the id is in an <a> tag.  We have seen poor autoscrolling behavior when the id is an a header tag, such that the header ends up obscured under the navigation bar at the top of the page.
You probably meant heading elements, because "header tag" is something different. Do you have any references those issues reports? Because in html5 the fragment identifiers are the only correct way to have internal document bookmarks [1] [2]. If some browsers do not navigate to fragment identifiers except for <a> element there must be bugs reported that  which will be fixed soon.
The html5 specification is very specific about navigating to the fragment identifier [3]. So, there should no be difference between navigating to "<a id=" or to any other element having id attribute. If you just need an extra vertical space above header you could use css style or <p>, but usage of <a> as an upper margin seems odd since it is a special tag.

--Semyon

[1] https://www.w3schools.com/html/html_links.asp
[2] http://www.html5-tutorials.org/html-basics/links/
[3] https://www.w3.org/TR/html5/browsers.html#scroll-to-fragid


-- Jon


On 10/23/2017 10:08 PM, Semyon Sadetsky wrote:
Hi Sergey,

I see no reason to have an extra empty anchor tag to set a bookmark. The id attribute works with any element.

For example:

    <a id="Definitions"></a>
    <h3>Definitions</h3>

should be

    <h3 id="Definitions">Definitions</h3>

--Semyon

On 10/23/2017 02:42 PM, Sergey Bylokhov wrote:

Hello,
Please review the fix for.
8182410: missing 'title' in api/javax/swing/plaf/synth/doc-files/componentProperties.html
8183508: multi_tsc.html should be updated
8181289: Invalid HTML 5 in AWT/Swing docs

Description:
 - Illegal characters were removed.
 - Unsupported tags/properties were removed -like <tt>, <center>, font, etc.(except the tags related to tables which I'll fix later).
 - HTML5 doctype is set for all files.
 - The <title> is set for all files.
 - <a name="" is replaced by <a id=""
Why you replace

 - Copyrights were added to some files.

Note that I placed a <head> tag before copyright to solve errors like:
"A charset attribute on a meta element found after the first 1024 bytes. Fatal Error: Changing encoding at this point would need non-streamable behavior"

specdiff: http://cr.openjdk.java.net/~serb/8181289/specdiff/overview-summary.html

Bugs:
    https://bugs.openjdk.java.net/browse/JDK-8182410
    https://bugs.openjdk.java.net/browse/JDK-8183508
    https://bugs.openjdk.java.net/browse/JDK-8181289

Webrev can be found at: http://cr.openjdk.java.net/~serb/8181289/webrev.00