[webkit-reviews] review denied: [Bug 28854] Text Fields and Text Areas are reported as read-only by inspect32.exe : [Attachment 38833] Fix + Removed Testing Code

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Brian Weinstein
<bweinstein at apple.com>'s request for review:
Bug 28854: Text Fields and Text Areas are reported as read-only by
inspect32.exe
https://bugs.webkit.org/show_bug.cgi?id=28854

Attachment 38833: Fix + Removed Testing Code
https://bugs.webkit.org/attachment.cgi?id=38833&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +    if (m_renderer->isTextField()) {
> +	   HTMLInputElement* input =
static_cast<HTMLInputElement*>(m_renderer->node());
> +	   return input->readOnly();
> +    } else if (m_renderer->isTextArea()) {
> +	   HTMLTextAreaElement* textArea =
static_cast<HTMLTextAreaElement*>(m_renderer->node());
> +	   return textArea->readOnly();
> +    }

We normally don't do else after if.

I think the code would read fine without a local variable.

    if (m_renderer->isTextField())
	return static_cast<HTMLInputElement*>(m_renderer->node())->readOnly()

Can this be tested with a regression test? I think there are
accessibility-driven regression tests. If so, please add a test.

review- because of the lack of a test. If it's not possible to test this, then
feel free to put up for review again.