[OpenJDK 2D-Dev] <AWT Dev> [10] Review Request: 8182410, 8183508, 8181289

Semyon Sadetsky semyon.sadetsky at oracle.com
Thu Nov 23 04:25:24 UTC 2017 

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
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20171122/a887916d/attachment-0001.html>

More information about the 2d-dev mailing list