Bug 249323 - Make Attr.value/nodeValue/textContent not nullable
Summary: Make Attr.value/nodeValue/textContent not nullable
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: BrowserCompat, InRadar
Depends on:
Blocks:
 
Reported: 2022-12-14 10:07 PST by Ahmad Saleem
Modified: 2023-07-05 09:24 PDT (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ahmad Saleem 2022-12-14 10:07:23 PST
Hi Team,

While going through Blink's commit, I noted another bug where we differ from other browsers:

Test Case - http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=3528

Blink Commit - https://chromium.googlesource.com/chromium/blink/+/a7c3bb9f22401c30e906a3ec53595247e707de83

*** Safari 16.2 / STP 159 ***

log: attr.value=null => getAttribute()=="null"
log: attr.nodeValue=null => getAttribute()==null
log: attr.textContent=null => getAttribute()==null
rendering mode: CSS1Compat
document has no title


*** Chrome Canary 110 & Firefox Nightly 110 ***

log: attr.value=null => getAttribute()=="null"

log: attr.nodeValue=null => getAttribute()==""

log: attr.textContent=null => getAttribute()==""

rendering mode: CSS1Compat

document has no title


_____

Just wanted to raise it for future purposes.

Thanks!
Comment 1 Ryosuke Niwa 2022-12-19 00:57:32 PST
It seems like a good change to merge.
Comment 2 Ahmad Saleem 2022-12-19 01:08:36 PST
(In reply to Ryosuke Niwa from comment #1)
> It seems like a good change to merge.

Cool! I will give it a go later today. Thanks for your input! I usually don't touch IDL files (fear of unknown). :-)
Comment 3 Radar WebKit Bug Importer 2022-12-21 10:08:37 PST
<rdar://problem/103602803>
Comment 4 Ahmad Saleem 2023-07-04 06:53:04 PDT
void Attr::setNodeValue(const String& value)
{
    const AtomString valueWithoutNull = value.isNull() ? emptyAtom() : AtomString(value);
    setValue(valueWithoutNull);
}

^ This fixes failing test case locally. Can do PR later. Thanks!
Comment 5 EWS 2023-07-05 09:24:16 PDT
Committed 265769@main (2da4c96001cf): <https://commits.webkit.org/265769@main>

Reviewed commits have been landed. Closing PR #15550 and removing active labels.