Last modified: 2012-11-26 17:28:00 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T44134, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 42134 - VisualEditor: Floated nodes don't get shields in Chrome
VisualEditor: Floated nodes don't get shields in Chrome
Status: RESOLVED FIXED
Product: VisualEditor
Classification: Unclassified
ContentEditable (Other open bugs)
unspecified
All All
: High normal
: VE-deploy-2012-11-26
Assigned To: Inez Korczyński
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-15 03:09 UTC by Roan Kattouw
Modified: 2012-11-26 17:28 UTC (History)
5 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Roan Kattouw 2012-11-15 03:09:28 UTC
If you have a floated table inside an alien, it gets a shield (and a phantom) in Firefox but not in Chrome. This is because the table is floated with external CSS triggered by class="infobox", rather than an inline style attribute. At the time when we check for $this.css('float'), $this hasn't been attached to the main DOM yet. Firefox computes styles on unattached nodes correctly, but Chrome doesn't.

The solution is to wait until after the entire CE rendering has been built and attached to the DOM before generating shields and phantoms. Possible implementations:

1. Add an onAttach function to ce.Node. After attaching, traverse the entire CE tree and invoke onAttach() on every node (bottom up). This is expensive, though, because you have to traverse the entire tree just to invoke onAttach on the few nodes that use it.

2. Make the ce.Surface throw an 'attached' event after attaching to the DOM, and make the ce.AlienBlockNode constructor listen for this event. This is not as clean, but more efficient, because only nodes that need processing are processed.

I have a slight preference for #2, but I can be convinced either way.

In both cases, the call to onUpdate() in the ce.AlienBlockNode constructor would be removed, otherwise the HTML would be set twice.
Comment 1 Inez Korczyński 2012-11-15 22:33:00 UTC
Fixed here: https://gerrit.wikimedia.org/r/#/c/33661/

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links