Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Rework code to not use class names #10133

Closed
bsclifton opened this issue Jul 25, 2017 · 0 comments · Fixed by #10165
Closed

Rework code to not use class names #10133

bsclifton opened this issue Jul 25, 2017 · 0 comments · Fixed by #10165

Comments

@bsclifton
Copy link
Member

bsclifton commented Jul 25, 2017

Test plan

#10165 (comment)


Discovered by @jonathansampson and @kevinlawler while troubleshooting #10029

Our current theory on the root cause:

The Aphrodite library just introduced a minify option (specifically with Khan/aphrodite@7f9f602). Unfortunately, we have checks which rely on the class name and we DO set NODE_ENV = production while doing a build (which triggers this option OR a similar option).

Example of problem

Here's an example of the issue, showing the difference in the same element between two versions of Brave (notice the class attribute is different):
0.17.16:

<div class="popupWindow_yfkq6x-o_O-reverseExpand_142fl4c" style="height: 459px; top: 42px; width: 352px; right: 1em;">...</div>

0.17.17:

<div class="_yfkq6x-o_O-_142fl4c" style="height: 459px; top: 44px; width: 352px; right: 1em;">...</div>

The proposed fix:

Update the following code to check using attributes, not class names:

// TODO(bsclifton): update this to use eventUtil.eventElHasAncestorWithClasses

@bsclifton bsclifton added this to the 0.18.x hotfix milestone Jul 25, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.