On Mon, 9 Dec 2024 09:56:15 GMT, ScientificWare <d...@openjdk.org> wrote:
>> This is referenced in Java Bug Database as >> - [JDK-8314731 : Adds support for the alt attribute in the image type input >> HTML tag.](https://bugs.java.com/bugdatabase/view_bug?bug_id=8314731) >> >> This is tracked in JBS as >> - [JDK-8314731 : Add support for the alt attribute in the image type input >> HTML tag](https://bugs.openjdk.java.net/browse/JDK-8314731) >> >> According [HTML 3.2 >> specification](https://www.w3.org/TR/2018/SPSD-html32-20180315/#input) >> >> `alt` is not an attribute of the `input` element. >> >> According [HTML 4.01 >> specifications](https://www.w3.org/TR/html4/interact/forms.html#h-17.4) : >> >>> ... For accessibility reasons, authors should provide [alternate >>> text](https://www.w3.org/TR/html4/struct/objects.html#alternate-text) for >>> the image via the >>> [alt](https://www.w3.org/TR/html4/struct/objects.html#adef-alt) attribute. >>> ... >> >> This feature is not implemented in `FormView.java`. >> >> ⚠️ ~~This also affects the HTML 32 DTD~~ >> >>  >> >> Left before the patch and right after the patch. >> >> >> import java.awt.BorderLayout; >> import java.awt.Dimension; >> import javax.swing.JEditorPane; >> import javax.swing.JFrame; >> import javax.swing.JScrollPane; >> import javax.swing.SwingUtilities; >> import javax.swing.text.Document; >> import javax.swing.text.html.HTMLEditorKit; >> import javax.swing.text.html.StyleSheet; >> >> public class HTMLAddsSupportAltInputTag { >> public static void main(String[] args) { >> new HTMLAddsSupportAltInputTag(); >> } >> >> public HTMLAddsSupportAltInputTag() { >> SwingUtilities.invokeLater(new Runnable(){ >> public void run(){ >> JEditorPane jEditorPane = new JEditorPane(); >> jEditorPane.setEditable(false); >> JScrollPane scrollPane = new JScrollPane(jEditorPane); >> HTMLEditorKit kit = new HTMLEditorKit(); >> jEditorPane.setEditorKit(kit); >> StyleSheet styleSheet = kit.getStyleSheet(); >> styleSheet.addRule(""" >> body { >> color: #000; >> font-family:times; >> margin: 4px; >> } >> """); >> String htmlString = """ >> <html> >> <body> >> <input type=image name=point >> src="file:oracle_logo_50x50.jpg" alt="Logo Oracle JPG"> >> <p> >> <input type=image name=point src="file:none_ora... > > ScientificWare has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 25 commits: > > - Merge master > - JDK-8314731 : Remove all indentations accidentally introduced by the > previous commit. > - Merge master > - Merge master > - jdk-8314731 : FormView Alt Support. > > FormView.java : > - revert ALL unrelated changing to formatting. > > bug8314731.java : > - Fix the test description. > - Change where the user interface is created. > - Add a finall block to be sure the Frame is disposed. > - Replace "testPassed" with "testFailed". > - Merge master > - Replaces this title with "alt attribute test in HTML image type input". > > Moves this test to /jdk/test/jdk/javax/swing/text/html. > - bug8314731.java : Corrects the CopyRight date. > - FormView.java : > Removes a whitespace > > bug8314731.java : > Adds a newline at end of file. > - getMaximumSpan(int axis) method > doc -> Not used > > mouseReleased(MouseEvent evt) method > elem and hdoc -> not used > return -> could be removed, method returns void > > loadElementDataIntoBuffer(Element elem, StringBuilder buffer) method > value != null -> name can't be null at this point > > getInputElementData(AttributeSet attr) method > value = null -> Already set at null > - ... and 15 more: https://git.openjdk.org/jdk/compare/69e664de...9b423808 Bump the copyright year to 2025 in both files. src/java.desktop/share/classes/javax/swing/text/html/FormView.java line 288: > 286: button = icon.getImageLoadStatus() == > MediaTracker.COMPLETE > 287: ? new JButton(icon) > 288: : new JButton(altAtt); Suggestion: button = icon.getImageLoadStatus() == MediaTracker.COMPLETE ? new JButton(icon) : new JButton(altAtt); I'd rather align the wrapped lines with `icon` as it's the start of the expression, it's one of the options in wrapping lines, and I think it nearly always looks better. test/jdk/javax/swing/text/html/bug8314731.java line 28: > 26: * @bug 8314731 > 27: * @summary FormView doesn't support the alt attribute > 28: * @key headful Suggestion: * @bug 8314731 * @key headful * @summary FormView doesn't support the alt attribute Usually, `@key` follows after `@bug`… for consistency with other tests. test/jdk/javax/swing/text/html/bug8314731.java line 50: > 48: private static boolean testPassed; > 49: private static JFrame jf; > 50: private static JEditorPane jEditorPane; Suggestion: private static JFrame frame; private static JEditorPane editorPane; I prefer not to have `j` from `JFrame` or `JEditorPane` in the name of a field, it doesn't add anything; `frame` is more descriptive than `jf`. test/jdk/javax/swing/text/html/bug8314731.java line 64: > 62: try { > 63: > SwingUtilities.invokeAndWait(bug8314731::createAndSetVisibleUI); > 64: testPassed = ContainsAlt(jEditorPane); Technically, `ContainsAlt` should be called on EDT, too. test/jdk/javax/swing/text/html/bug8314731.java line 66: > 64: testPassed = ContainsAlt(jEditorPane); > 65: } finally { > 66: SwingUtilities.invokeAndWait(new Runnable() { Put this code directly into the `main` method instead of using `bug8314731` constructor. Taking other comments into account, I expect the `main` method to look like this: public static void main(String[] args) throws Exception { try { SwingUtilities.invokeAndWait(bug8314731::createAndSetVisibleUI); SwingUtilities.invokeAndWait(() -> { if (!ContainsAlt(jEditorPane)) { throw new RuntimeException("FormView doesn't support the alt attribute."); } }); } finally { SwingUtilities.invokeAndWait(() -> { if (jf != null) { jf.dispose(); } }); } } Declare `containsAlt` static to be able to call it directly from `main`, remove the `testPassed` field and the `bug8314731` constructor. test/jdk/javax/swing/text/html/bug8314731.java line 76: > 74: private static void createAndSetVisibleUI() { > 75: > 76: jEditorPane = new JEditorPane(); Suggestion: private static void createAndSetVisibleUI() { jEditorPane = new JEditorPane(); There's no need for a blank line as the first line of a method. test/jdk/javax/swing/text/html/bug8314731.java line 87: > 85: body { > 86: color: #000; > 87: font-family:times; Suggestion: font-family: times; Be consistent, use a space after the colon in CSS rules. test/jdk/javax/swing/text/html/bug8314731.java line 111: > 109: } > 110: > 111: private boolean ContainsAlt(Container container) { Suggestion: private boolean containsAlt(Container container) { Method names starts with lower-case letter. test/jdk/javax/swing/text/html/bug8314731.java line 117: > 115: if (text.equals("Logo")) { > 116: return true; > 117: } Suggestion: if (c instanceof JButton butt) { return "Logo".equals(butt.getText()); I think this can be simplified — there's only one `JButton`, so if its text isn't `"Logo"` return `false` right away. test/jdk/javax/swing/text/html/bug8314731.java line 121: > 119: if (ContainsAlt(cont)) { > 120: return true; > 121: } Suggestion: return ContainsAlt(cont)); There's no need for the `if`-statement here, too. ------------- Changes requested by aivanov (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/15319#pullrequestreview-3151374154 PR Review Comment: https://git.openjdk.org/jdk/pull/15319#discussion_r2298053885 PR Review Comment: https://git.openjdk.org/jdk/pull/15319#discussion_r2298130136 PR Review Comment: https://git.openjdk.org/jdk/pull/15319#discussion_r2298063946 PR Review Comment: https://git.openjdk.org/jdk/pull/15319#discussion_r2298119438 PR Review Comment: https://git.openjdk.org/jdk/pull/15319#discussion_r2298066861 PR Review Comment: https://git.openjdk.org/jdk/pull/15319#discussion_r2298068175 PR Review Comment: https://git.openjdk.org/jdk/pull/15319#discussion_r2298069878 PR Review Comment: https://git.openjdk.org/jdk/pull/15319#discussion_r2298081897 PR Review Comment: https://git.openjdk.org/jdk/pull/15319#discussion_r2298081308 PR Review Comment: https://git.openjdk.org/jdk/pull/15319#discussion_r2298083692
