Combining div with ID containing '?' and phx-update="ignore" throughs Floki error

[nathanmalishev]

Environment

• Phoenix LiveView version (mix deps): Introduced 20.0.7 -> 20.0.8

Actual behavior

Any div in combination with an ID containing '?' along with 'phx-update=ignore' will throw a Floki error, only when running tests

Throws error -> <div id="test?param=1" phx-update="ignore"></div>
Does not throw error -> <div id="test?param=1"></div>

Example repo -> https://github.com/nathanmalishev/dom_id_phx_ignore_test/blob/d1d0243851d141527d93ec2155bd185b919ad239/lib/dom_id_phx_ignore_web/live/article_live/index.html.heex#L10

 ** (FunctionClauseError) no function clause matching in Floki.Selector.Parser.do_parse/2

          The following arguments were given to Floki.Selector.Parser.do_parse/2:

              # 1
              [{:equal, 1}, {:identifier, 1, ~c"1"}]

              # 2
              %Floki.Selector{id: "test", type: "param", classes: [], attributes: [], namespace: nil, pseudo_classes: [], combinator: nil}

          Attempted function clauses (showing 10 out of 21):

              defp do_parse([], selector)
              defp do_parse([{:close_parentesis, _} | t], selector)
              defp do_parse([{:comma, _} | t], selector)
              defp do_parse([{:identifier, _, namespace}, {:namespace_pipe, _} | t], selector)
              defp do_parse([{:identifier, _, type} | t], selector)
              defp do_parse([{~c"*", _} | t], selector)
              defp do_parse([{:hash, _, id} | t], selector)
              defp do_parse([{:class, _, class} | t], selector)
              defp do_parse([{~c"[", _} | t], selector)
              defp do_parse([{:pseudo_not, _} | t], selector)
              ...
              (11 clauses not shown)

          stacktrace:
            (floki 0.35.4) lib/floki/selector/parser.ex:40: Floki.Selector.Parser.do_parse/2
            (floki 0.35.4) lib/floki/selector/parser.ex:36: Floki.Selector.Parser.do_parse_all/2
            (floki 0.35.4) lib/floki/finder.ex:21: Floki.Finder.find/2
            (phoenix_live_view 0.20.11) lib/phoenix_live_view/test/dom.ex:572: Phoenix.LiveViewTest.DOM.by_id/2
            (phoenix_live_view 0.20.11) lib/phoenix_live_view/test/dom.ex:447: Phoenix.LiveViewTest.DOM.apply_phx_update/4
            (floki 0.35.4) lib/floki/traversal.ex:6: anonymous fn/3 in Floki.Traversal.traverse_and_update/2
            (floki 0.35.4) lib/floki/traversal.ex:18: Floki.Traversal.traverse_and_update/3
            (floki 0.35.4) lib/floki/traversal.ex:23: Floki.Traversal.traverse_and_update/3
            (floki 0.35.4) lib/floki/traversal.ex:37: Floki.Traversal.traverse_and_update/3
            (floki 0.35.4) lib/floki/traversal.ex:18: Floki.Traversal.traverse_and_update/3
            (floki 0.35.4) lib/floki/traversal.ex:37: Floki.Traversal.traverse_and_update/3
            (floki 0.35.4) lib/floki/traversal.ex:18: Floki.Traversal.traverse_and_update/3
            (floki 0.35.4) lib/floki/traversal.ex:23: Floki.Traversal.traverse_and_update/3
            (floki 0.35.4) lib/floki/traversal.ex:6: Floki.Traversal.traverse_and_update/2
            (phoenix_live_view 0.20.11) lib/phoenix_live_view/test/dom.ex:305: Phoenix.LiveViewTest.DOM.patch_id/4
            (phoenix_live_view 0.20.11) lib/phoenix_live_view/test/client_proxy.ex:656: Phoenix.LiveViewTest.ClientProxy.patch_view/4
            (phoenix_live_view 0.20.11) lib/phoenix_live_view/test/client_proxy.ex:156: Phoenix.LiveViewTest.ClientProxy.init/1
            (stdlib 5.2) gen_server.erl:980: :gen_server.init_it/2
            (stdlib 5.2) gen_server.erl:935: :gen_server.init_it/6
            (stdlib 5.2) proc_lib.erl:241: :proc_lib.init_p_do_apply/3

Expected behavior

Believe this shouldn't occur as there's no HTML restrictions around '?' in div ids?

I was using this for ease of reference in testing to just assert that the src links were on the page as they should be.

[josevalim]

I think we are using the id for a query selector and it has invalid characters. We may need to escape it. Or it is a Floki bug. :D

[SteffenDE]

I think we are using the id for a query selector and it has invalid characters. We may need to escape it. Or it is a Floki bug. :D

Yes. Instead of ##{id} we need to use %Floki.Selector{id: id}. I‘m just not sure because the Floki.Selector module is not documented and there’s no other way to construct an ID selector or escape strings in selectors.

[josevalim]

cc @philss

[chrismccord]

We may be able to change our query to [id="#{id}"]

[ypconstante]

Browsers also fail with this id

> document.querySelectorAll('#test?param=1')
Uncaught DOMException: Document.querySelectorAll: '#test?param=1' is not a valid selector

Changing this to use attribute selector has a visible performance impact, specially considering how frequent this is called during the tests. Maybe we could do this slower search only for ids with special characters.

##### With input big #####
Name                   ips        average  deviation         median         99th %
id                  587.73        1.70 ms     ±4.75%        1.69 ms        1.93 ms
id attribute        385.74        2.59 ms     ±7.75%        2.56 ms        3.22 ms

Comparison: 
id                  587.73
id attribute        385.74 - 1.52x slower +0.89 ms

Memory usage statistics:

Name            Memory usage
id                   1.87 MB
id attribute         2.93 MB - 1.57x memory usage +1.06 MB

**All measurements for memory usage were the same**

##### With input medium #####
Name                   ips        average  deviation         median         99th %
id                  1.77 K      564.48 μs    ±11.78%      557.67 μs      764.69 μs
id attribute        1.23 K      816.07 μs     ±6.06%      792.34 μs      990.05 μs

Comparison: 
id                  1.77 K
id attribute        1.23 K - 1.45x slower +251.59 μs

Memory usage statistics:

Name            Memory usage
id                 621.45 KB
id attribute       998.65 KB - 1.61x memory usage +377.20 KB

**All measurements for memory usage were the same**

##### With input small #####
Name                   ips        average  deviation         median         99th %
id                  8.04 K      124.45 μs    ±25.72%      115.63 μs      234.85 μs
id attribute        5.30 K      188.52 μs    ±11.77%      179.34 μs      266.56 μs

Comparison: 
id                  8.04 K
id attribute        5.30 K - 1.51x slower +64.08 μs

Memory usage statistics:

Name            Memory usage
id                 127.27 KB
id attribute       217.66 KB - 1.71x memory usage +90.39 KB

[SteffenDE]

@ypconstante in the browser one could use document.getElementById instead. Floki doesn't have a function like that. What do you think about %Floki.Selector{id: id}?

[josevalim]

Maybe we should contribute a find_by_id function upstream to Floki?

[philss]

Maybe we should contribute a find_by_id function upstream to Floki?

Sounds good to me :)