On Mon Jun 16 08:28:23 2025 +0000, Jacek Caban wrote:
Why is this such a focus? Honestly. While this MR improves compatibility with older compatibility modes, I think it could be skipped entirely if the goal is to implement IE9+ attribute behavior. This isn’t observable from scripts, right? A quick test suggests these attributes haven’t been exposed through the collection since IE8. My guess (though I haven’t tested it) is that we should be using `IHTMLElement5::ie8_attributes` in those compat modes. Your test suggests we likely need two separate collections: one that exposes these built-in attributes and one that doesn’t. The latter is what matters for IE9+ (and IE8), yet your changes seem to be focused on the former. The current attribute collection is a frustrating piece of legacy code. It's essentially a workaround that we need for old compatibility modes that don’t treat HTML attributes like proper DOM attributes. If we can avoid taking those code paths in standards-compliant modes, that would be ideal. In the best case, the collection should be a thin wrapper around `nsIDOMMozNamedAttrMap`, and the attribute object a thin wrapper around `nsIDOMAttr`. Why can’t we structure it that way?
Well I mean I thought we weren't supposed to go with half-baked solutions upstream and should focus on proper implementation? Missing features are fine of course, but what you suggest is a complete dead end in this case (i.e. it won't be improved in the future without completely removing it and replacing it with something else) for several reasons:
1. get_attributes still needs it (from COM caller, not scripts) 2. With this architecture, having one that exposes builtins and one that doesn't is basically just a flag change, not a completely different way of looking them up. Furthermore we *also* needs this for IE8 ideally (not just IE9+). 3. How do you deal with DISPIDs assigned in the collection if you just use gecko wrapper for it? 4. You can mix legacy attr nodes with IE9+ nodes in the same collection, this applies to both legacy collections and IE9+ collections (legacy collections can hold both legacy nodes and IE9+ nodes, and same with IE9+ collections). So, having the list (current impl) work for both is probably the way to go.
Furthermore, while it seems nice to have just thin wrappers, we'd still end up having two separate implementations, and I don't think that's better, is it?
So I want to get it right since in Proton it's just a hack with wrong behavior in many places. At the very least, not some thing that will need a redesign if ever needed because it's a dead end...