Bug 78751 - Add support for the translate attribute in html elements.
Summary: Add support for the translate attribute in html elements.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-15 15:09 PST by Pablo Flouret
Modified: 2012-04-11 21:29 PDT (History)
6 users (show)

See Also:


Attachments
Patch (18.82 KB, patch)
2012-02-15 15:25 PST, Pablo Flouret
no flags Details | Formatted Diff | Diff
Fixed changelogs and added assert (19.62 KB, patch)
2012-02-15 15:58 PST, Pablo Flouret
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Pablo Flouret 2012-02-15 15:25:09 PST
Created attachment 127249 [details]
Patch
Comment 2 Adam Barth 2012-02-15 15:43:20 PST
Comment on attachment 127249 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=127249&action=review

Is there any more to this attribute?  A link to the spec would be helpful so I can read up on it easily.  Some nits below.

> Source/WebCore/ChangeLog:8
> +        Add support for the translate attribute in html elements.
> +        https://bugs.webkit.org/show_bug.cgi?id=78751
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Test: fast/dom/HTMLElement/translate.html

Can you add some more information to the ChangeLog, like a link to the spec?

> Source/WebCore/html/HTMLElement.cpp:823
> +            if (mode != TranslateAttributeInherit)
> +                return mode == TranslateAttributeYes;

Should we ASSERT that the mode is either TranslateAttributeYes or TranslateAttributeNo in the body of this if?
Comment 3 Pablo Flouret 2012-02-15 15:50:09 PST
(In reply to comment #2)
> (From update of attachment 127249 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=127249&action=review
> 
> Is there any more to this attribute?  A link to the spec would be helpful so I can read up on it easily.  Some nits below.

The link to the spec is in the description field of this bug, to wit, http://www.whatwg.org/specs/web-apps/current-work/multipage/elements.html#attr-translate

I forgot to add it to the changelog, will do.

> 
> > Source/WebCore/html/HTMLElement.cpp:823
> > +            if (mode != TranslateAttributeInherit)
> > +                return mode == TranslateAttributeYes;
> 
> Should we ASSERT that the mode is either TranslateAttributeYes or TranslateAttributeNo in the body of this if?

We could.
Comment 4 Pablo Flouret 2012-02-15 15:58:23 PST
Created attachment 127260 [details]
Fixed changelogs and added assert
Comment 5 Adam Barth 2012-02-15 16:05:01 PST
Comment on attachment 127260 [details]
Fixed changelogs and added assert

Thanks!
Comment 6 WebKit Review Bot 2012-02-15 20:40:14 PST
Comment on attachment 127260 [details]
Fixed changelogs and added assert

Rejecting attachment 127260 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
ools/ChangeLog
CONFLICT (content): Merge conflict in Tools/ChangeLog
Failed to merge in the changes.
Patch failed at 0001 [chromium] Rebaseline JPEG image results after r107389

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 164.

Full output: http://queues.webkit.org/results/11527988
Comment 7 WebKit Review Bot 2012-02-15 21:42:23 PST
Comment on attachment 127260 [details]
Fixed changelogs and added assert

Clearing flags on attachment: 127260

Committed r107883: <http://trac.webkit.org/changeset/107883>
Comment 8 WebKit Review Bot 2012-02-15 21:42:28 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 Simon Fraser (smfr) 2012-03-26 14:21:46 PDT
This broke mobile.orange.fr: bug 82246.
Comment 10 rakesh makluri 2012-04-11 21:29:55 PDT
Hi Adam,

  With this support our product is breaking now. Present we are loading and using the translate() API on HTML elements to translate. But now with this change it is breaking. Is there any way to override this attribute.

  We can change the API but, so many existing customers and our internal components are also using this.
  Please help with this.